Wait, is that variable ever assigned?

One of the inspections we wanted to implement in Rubberduck for the next release, is one that finds all unassigned variables in a VBA project.

Implementing this inspection has been ...complicated (it's the most complex inspection in the Rubberduck code base so far). Here's the code:

```
public class VariableNotAssignedInspection : IInspection
{
public VariableNotAssignedInspection()
{
Severity = CodeInspectionSeverity.Error;
}

public string Name { get { return InspectionNames.VariableNotAssigned; } }
public CodeInspectionType InspectionType { get { return CodeInspectionType.CodeQualityIssues; } }
public CodeInspectionSeverity Severity { get; set; }

public IEnumerable GetInspectionResults(IEnumerable parseResult)
{
var parseResults = parseResult.ToList();

// publics & globals delared at module-scope in standard modules:
var globals =
parseResults.Select(result =>
new
{
Name = result.QualifiedName,
Globals = result.ParseTree.GetContexts(new DeclarationSectionListener())
.OfType()
.Where(context =>
context.visibility() != null &&
context.visibility().GetText() != Tokens.Private)
.SelectMany(context => context.variableListStmt().variableSubStmt()
.Select(variable => variable.ambiguousIdentifier()))
})
.SelectMany(module => module.Globals.Select(global =>
new
{

This doesn't address your actually question (simplifying the logic/implementation), but I feel like it's important.

For the sake of readability, I would recommend replacing some of the nested lambdas with a function reference to a named methods. The assignment to globals in the first section of code has 5 levels of indentation. That is a bit much for an assignment statement. Looking at the lambda passed to the first Select() is scary. If I had more context with the code, it might not seem that daunting. But at first glance, there is a lot going on.

The way the code is now, I feel like I would have to dive in and understand all the details just to understand the basic steps it is taking. The first time I read through code, I don't care about the small implementation details. A well named method is a great way to extract away the details and convey your intent. Then, when I have a higher level understanding, then I can dive into the things that caught my eye as potential issues. Requiring this much up-front commitment is likely hindering your actual question from being looked into.

A simple example from your code:

.Where(context =>
       context.visibility() != null &&
       context.visibility().GetText() != Tokens.Private)

I just want to know you are filtering for non-private tokens, not how you check for one.

.Where(IsNonPrivateToken)

This takes the pressure of the reader as the LINQ statement reads more like a sentence than code.

Doing this will also help with the excessive indentation. The majority of that first Select() lambda requires horizontal scrolling.

StackExchange Code Review Q#80641, answer score: 8