MA0089 also warns for preceding lines in a multi-line call chain about meziantou.analyzer

[meziantou]

The diagnostic is reported on the operation. In this case this is an IInvocationOperation, so the associated syntax is the InvocationExpressionSyntax. This includes the target (everything before (, not just the method name) and the arguments. Here's an example of what it means:

_ = "test".Substring(0, 1).Replace(" ", " ").Trim()
    ^--------------------------------------^

I don't know what the standard way is, but this is the easiest! If you want to change the reported location, there are multiple impacts:

• The analyzer must start dealing with the syntax API instead of using the Operation API (the generic way). This means your code is now specific for C# or VB.NET and is more subject to bugs and issues when a new version of the language is released.
• The fixer needs to find the full context again instead of having the right expression immediately

So, this increases the code complexity.

[Piedone]

Thanks for your quick reply!

I really don't know enough about how analyzers work internally to comment. As a user, I found the demonstrated behavior confusing; I actually dug around for a bit before figuring out that in fact, there are no issues in the preceding lines. If it would be disproportionally hard to fix this behavior then please disregard it.

[meziantou]

I don't think I'll change the reported location, unless the change is easier than I think. Note, that I can also improve the error message to include the replaced characters. This would reduce the ambiguity.

By curiosity, which location do you expect to be reported? The method name? The arguments? The method name and arguments? Something else?

[Piedone]

The message including the characters would help a lot.

I'd expect the squiggly lines to appear only on the line of the violation, i.e. the 4th line in the example (and if the whole statement would be a single line then also only under the offending Replace(), though I guess that's stretching what's feasible).

[meziantou]

After looking at the code,

• I've updated the reported location of methods with a single argument (StartsWith, EndsWith, IndexOf, LastIndexOf, Join).
• Replace will still report the full invocation. I think it would be possible to report both arguments using "AdditionalLocation", but I've never use them, so I'm not sure how it will behave in VS or Rider.
• I've implemented a CodeFixer, so it's easy to apply the fix (and view the diff before applying the fix).

[Piedone]

Thank you! Didn't you also want to update the error message?

[Piedone]

Also, in the above example, the code fix will produce this code:

        var replaced = "test"
            .Replace("string 1", "string 2", StringComparison.Ordinal)
            .Replace("string 3", "string 4", StringComparison.Ordinal)
            .Replace('\n', 'n', StringComparison.Ordinal);

This is invalid, since string Replace(char oldChar, char newChar) doesn't have an overload with a StringComparison parameter.

[meziantou]

Thanks for reporting the bug. I've improved the tests to detect wrong fixes. The new flow for all tests is:

• Compile the source code to validate it is valid
• Run analyzers and validate reported locations
• Apply code fixes if any
• (new) Compile the fixed code
• Validate the fixed code is the one expected

I've detected bugs in 3 code fixers!

Didn't you also want to update the error message

Nope, but the diagnostic is now reported at the method name location.

[Piedone]

Thank you for the quick fix!

I see, I did look for the reporting location changing, but didn't notice anything, after multiple rebuild/clean cycles even, thinking that VS didn't notice the analyzer package being updated. Now after the v2.0.58 update it works,thank you!