Passing parameters by reference? Let me fix that for you

I've implemented a code inspection that verifies whether a procedure has parameters that are implicitly passed by reference - in VBA a parameter is passed ByRef unless it's explicitly specified ByVal.

Here is the implementation in question:

[ComVisible(false)]
public class ImplicitByRefParameterInspection : IInspection
{
    public ImplicitByRefParameterInspection()
    {
        Severity = CodeInspectionSeverity.Warning;
    }

    public string Name { get { return "Parameter is passed ByRef implicitly"; } }
    public CodeInspectionType InspectionType { get { return CodeInspectionType.CodeQualityIssues; } }
    public CodeInspectionSeverity Severity { get; set; }

    public IEnumerable GetInspectionResults(SyntaxTreeNode node)
    {
        var procedures = node.FindAllProcedures().Where(procedure => procedure.Parameters.Any(parameter => !string.IsNullOrEmpty(parameter.Instruction.Value)));
        var targets = procedures.Where(procedure => procedure.Parameters.Any(parameter => parameter.IsImplicitByRef)
                                                && !procedure.Instruction.Line.IsMultiline);

        return targets.SelectMany(procedure => procedure.Parameters.Where(parameter => parameter.IsImplicitByRef)
            .Select(parameter => new ImplicitByRefParameterInspectionResult(Name, parameter.Instruction, Severity)));
    }
}

Would there be a better way to do this? Here is the ImplicitByRefParameterInspectionResult class (gosh that's a long name!):

```
[ComVisible(false)]
public class ImplicitByRefParameterInspectionResult : CodeInspectionResultBase
{
public ImplicitByRefParameterInspectionResult(string inspection, Instruction instruction, CodeInspectionSeverity type)
: base(inspection, instruction, type)
{
}

public override IDictionary> GetQuickFixes()
{
return !Handled
? new Dictionary>
{
{"Pass parameter by value", PassParameterByVal},

Starting from this

var procedures = node.FindAllProcedures().Where(procedure => procedure.Parameters.Any(parameter => !string.IsNullOrEmpty(parameter.Instruction.Value)));
var targets = procedures.Where(procedure => procedure.Parameters.Any(parameter => parameter.IsImplicitByRef)
                                        && !procedure.Instruction.Line.IsMultiline);

return targets.SelectMany(procedure => procedure.Parameters.Where(parameter => parameter.IsImplicitByRef)
    .Select(parameter => new ImplicitByRefParameterInspectionResult(Name, parameter.Instruction, Severity)));

Let's collapse it into one expression

return node.FindAllProcedures()
    .Where(procedure => procedure.Parameters.Any(parameter => !string.IsNullOrEmpty(parameter.Instruction.Value)))
    .Where(procedure => procedure.Parameters.Any(parameter => parameter.IsImplicitByRef)
                                        && !procedure.Instruction.Line.IsMultiline)
    .SelectMany(procedure => procedure.Parameters.Where(parameter => parameter.IsImplicitByRef)
    .Select(parameter => new ImplicitByRefParameterInspectionResult(Name, parameter.Instruction, Severity)));

Move the last Select out of SelectMany

return node.FindAllProcedures()
    .Where(procedure => procedure.Parameters.Any(parameter => !string.IsNullOrEmpty(parameter.Instruction.Value)))
    .Where(procedure => procedure.Parameters.Any(parameter => parameter.IsImplicitByRef)
                                        && !procedure.Instruction.Line.IsMultiline)
    .SelectMany(procedure => procedure.Parameters)
    .Where(parameter => parameter.IsImplicitByRef)
    .Select(parameter => new ImplicitByRefParameterInspectionResult(Name, parameter.Instruction, Severity));

Now it seems like procedure.Parameters.Any(parameter => parameter.IsImplicitByRef) is redundant, so let's remove that and join the successive Wheres

return node.FindAllProcedures()
    .Where(procedure => procedure.Parameters.Any(parameter => !string.IsNullOrEmpty(parameter.Instruction.Value))
                        && !procedure.Instruction.Line.IsMultiline)
    .SelectMany(procedure => procedure.Parameters)
    .Where(parameter => parameter.IsImplicitByRef)
    .Select(parameter => new ImplicitByRefParameterInspectionResult(Name, parameter.Instruction, Severity));

It's not clear why procedure.Parameters.Any(parameter => !string.IsNullOrEmpty(parameter.Instruction.Value)) is necessary -- if it is, a comment is required, otherwise we can just write

return node.FindAllProcedures()
    .Where(procedure => !procedure.Instruction.Line.IsMultiline)
    .SelectMany(procedure => procedure.Parameters)
    .Where(parameter => parameter.IsImplicitByRef)
    .Select(parameter => new ImplicitByRefParameterInspectionResult(Name, parameter.Instruction, Severity));

Which is looking more much manageable. Or if you prefer the alternative syntax

return from procedure in node.FindAllProcedures()
       where !procedure.Instruction.Line.IsMultiline
       from parameter in procedure.Parameters
       where parameter.IsImplicitByRef
       select new ImplicitByRefParameterInspectionResult(Name, parameter.Instruction, Severity);

StackExchange Code Review Q#74005, answer score: 17