-1

Not being able to mark a parameter of a method as readonly so that it is impossible to be reassigned in the method, I started thinking abount creating an analyzer for this. The parameter would be attributed with

[AttributeUsage(AttributeTargets.Parameter)]
public class ReadOnlyAttribute: Attribute
{
  // ...
}

and the signature of a method would be

public class Foo
{
  public void Bar([ReadOnly] MyObject o)
  {
    o.DoSomething()    // this is OK
    o = new MyObject() // this would be flagged by the analyzer
  }
}

What I have so far is an DiagnosticAnalyzer class with these members:

[DiagnosticAnalyzer(LanguageNames.CSharp)]
internal class MyDiagnosticAnalyzer : DiagnosticAnalyzer
{
  private static readonly DiagnosticDescriptor Descriptor =
    new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, Category, DiagnosticSeverity.Info, true, Description, HelpLink);

  public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Descriptor);

  public override void Initialize(AnalysisContext context)
  {
    context.RegisterSymbolAction(AnalyzeSymbol, SymbolKind.Method);
  }

  private static void AnalyzeSymbol(SymbolAnalysisContext context)
  {
    var methodSymbol = context.Symbol as IMethodSymbol;
    var parameters = methodSymbol.Parameters;

    foreach (var p in parameters)
    {
      var attr = p.GetAttributes();

      foreach (var a in attr)
      {
        if (a.AttributeClass.Name == "ReadOnlyAttribute")
        {
          // now I have to find all references of p in order to check if any of them is a assignment
        }
      }
    }
  }
}

How do I find all reference of a parameter within the method body? And how do I know which one of them are assigments?

David
  • 2,426
  • 3
  • 24
  • 36
  • Did you try to `public override void Initialize(AnalysisContext context) { context.RegisterSymbolAction(AnalyzeSymbol, SymbolKind.Method) } private static void AnalyzeSymbol(SymbolAnalysisContext context) { var methodSymbol = context.Symbol as IMethodSymbol; // Analyze method symbols its params and many also ... }`? P.S. I can write it as answer if you want – George Alexandria Jun 23 '17 at 19:10
  • @GeorgeAlexandria See my edit. – David Jun 23 '17 at 19:43
  • I sugges you get SyntaxNode for current IMethodSymbol if it has "ReadOnlyAttribute" parameter then get all child nodes from SyntaxNode that are SimpleAssigmentStatement. So, you only need to compare statement' identifier and your parameter. `//IMethodSymbol method = null; var location = method.Locations.FirstOrDefault(); if (location == null) { // throw or return } var currentMethodNode = location.SourceTree.GetCompilationUnitRoot().FindNode(location.SourceSpan); // get SimpleAssignmentExpressions` – George Alexandria Jun 23 '17 at 20:26
  • @GeorgeAlexandria Can you please write it as an answer, so that it becomes clearer what you suggest. – David Jun 24 '17 at 06:56
  • See the answer. – George Alexandria Jun 24 '17 at 11:23
  • @David Did you end up implementing something you can share? I'm interested in it as well. – Andrew D. Bond Aug 01 '22 at 12:20
  • @AndrewD.Bond I am really surprised that this is already 5 years old - how time passes fast. I am sorry to tell you that I didn't continue on this since then because all in all it was a challenge, yet too cumbersome to finish. – David Aug 05 '22 at 09:36

1 Answers1

1

I sugges you get SyntaxNode for current IMethodSymbol if it has "ReadOnlyAttribute" parameter then get all descendant nodes from SyntaxNode that are AssignmentExpressionSyntax. So, you only need to compare statement' identifiers and your parameter. Give me know if you have some questions.

...
var methodSymbol = context.Symbol as IMethodSymbol;;
var parameters = methodSymbol.Parameters;
if (!parameters.SelectMany(p => p.GetAttributes()).Any(s => s.AttributeClass.Name == "ReadOnlyAttribute"))
{
    // So you don't have params with ReadOnly attribute
    return;
}

// So you have params with ReadOnly attribute
var location = methodSymbol.Locations.FirstOrDefault();
if (location == null)
{
    // throw or return 
}

// Can cahce CompilationRoot
var methodNode = location.SourceTree.GetCompilationUnitRoot().FindNode(locati‌​on.SourceSpan);
var childNodes = methodNode.ChildNodes().ToList();
// Expression-bodied memeber
var blockNode = childNodes.FirstOrDefault(s => s is BlockSyntax) ?? childNodes.FirstOrDefault(s => s is ArrowExpressionClauseSyntax);
if (blockNode == null)
{
    // throw or return 
}

// You can convert this linq to foreach and improve performance removing a where functions 
var assignmentIndetifiers = blockNode.DescendantNodes()
    .Select(s => s as AssignmentExpressionSyntax)
    .Where(s => s != null)
    .Select(s => s.Left as IdentifierNameSyntax)
    .Where(s => s != null)
    .Select(s => s.Identifier)
    .ToList();

// You retrive all left identifiers in assignment expressions from current block syntax and its descendant nodes
// You only need to check if assignmentIdentifiers contains your readonly parameter
// For example, you can compare by name (you can use custom equality comparer)
var names = assignmentIndetifiers.ToLookup(s => s.ValueText, EqualityComparer<string>.Default);
foreach (var parameter in parameters)
{
    if (names.Contains(parameter.Name))
    {
        foreach (var token in names[parameter.Name])
        {
            // throw that readonly argument is a assignment
            context.ReportDiagnostic(Diagnostic.Create(rule, token.GetLocation()));
        }
    }
}
George Alexandria
  • 2,841
  • 2
  • 16
  • 24
  • `var names = assignmentIndetifiers.ToDictionary(s => s.ValueText, EqualityComparer.Default);` throws an exception when I have multiple assignments to the same parameter in the method body. Even though the analyzer rule is there to prevent this, the implementation must cater for. – David Jun 25 '17 at 21:42
  • @David you can use Lookup instead of Dictionary) Do you want that I edit the answer?) – George Alexandria Jun 25 '17 at 21:47
  • I was hoping that a call to `.Distinct()` before the `.ToList()` would do but unfortunately not. – David Jun 25 '17 at 21:49
  • 1
    @David of course not, because you `Distinct` a SyntaxNodes that are not equal by definition. You can try `Distinct` with a custom `IEqualityComparer` for SyntaxNodes that compares they by `ValueText` – George Alexandria Jun 25 '17 at 21:53
  • 1
    Only looking at `AssignmentExpressionSyntax` is wrong. There are more ways to set a variable, e.g. passing it to a method as a `ref` or `out` parameter. I Recommend that you use the `SemanticModel.AnalyzeDataFlow(method.Body as SyntaxNode ?? method.ExpressionBody.Expression).WrittenInside`. – Kris Vandermotten Jun 27 '17 at 17:29
  • @KrisVandermotten I don't agree with you because passing readonly argument as a parameter to another method is correct case for current method, so the method that are invoked should check that it correct use the passing readonly parameter not the method that invoke it. – George Alexandria Jun 27 '17 at 17:35
  • @David I edited the answer, so please check it because usage of `Dictionary> names = assignmentIndetifiers.GroupBy(s => s.ValueText).ToDictionary(gr => gr.Key, gr => new List(gr))` it's less clearity and simplicity than usage of Lookup – George Alexandria Jun 27 '17 at 17:46
  • @KrisVandermotten @KrisVandermotten but I agree that it need look additionally at `ExpressionStatementSyntax` – George Alexandria Jun 27 '17 at 17:51
  • Passing a variable into a method as an out parameter means the variable will be written. Passing it as a ref parameter means it might be written, unless the ref parameter is marked [ReadOnly] as well. – Kris Vandermotten Jun 27 '17 at 17:56
  • @KrisVandermotten out – yes, it will be written I agree, ref – no, it might be written so in this case it can give a false alarm. Okay it can give a some warning not error, if so, I agree. `SemanticModel.AnalyzeDataFlow` will be more simplify than handling all of cases for `ExpressionStatement`, `ArgumentSyntax` etc. – George Alexandria Jun 27 '17 at 18:08
  • How do I check if an invocation with a ref argument (`InvocationExpressionSyntax`) has that parameter attributed with `[ReadOnly]` where the method is declared? If I am not mistaken I must get somehow from `InvocationExpressionSyntax` to an `ISymbol`. This seems possible for [fields](https://stackoverflow.com/questions/27848576/how-to-get-a-roslyn-fieldsymbol-from-a-fielddeclarationsyntax-node) but `InvocationExpressionSyntax.Declaration` does not exist as it does with `FieldDeclarationSyntax`. – David Jun 29 '17 at 04:57
  • @David you can find `ArgumentSyntax` (`InvocationExpressionSyntax` contains collection of `ArgumentSyntax`), next you need to analyze a `ArgumentSyntax.RefOrOutKeyword` and check that `ArgumentSyntax.Expression` contains `IdentifierNameSyntax` with your `ReadOnlyAttribute` – George Alexandria Jun 29 '17 at 16:46
  • @GeorgeAlexandria In my understanding this would be only possible if the parameter in the invocation would be marked with `[ReadOnly]` which certainly is not the case, because attribution on arguments is part of the signature of the declaring method. Alternatively please post a working code snippet - should I have missed something. – David Jun 29 '17 at 18:56
  • @David I answered to your question [here](https://stackoverflow.com/a/44834196/5359302) – George Alexandria Jun 29 '17 at 20:15