A region with the word generated anywhere in the title causes sonarlint to skip analysis for the entire file.
namespace SonarLint_Analysis_Failure
{
public static class ShadowedMember
{
public static int MyProperty { get; set; }
public class Initializer
{
public int MyProperty { get; set; } // S3218 should be thrown here
}
#region Requests generated from remote systems need to be tested here
#endregion
}
}
Hi @nnpcYvIVl,
After some code digging and discussion with Product Managers, we decided not to do any fix for the time being.
FYI this hack was put in place as an easy workaround for some legacy WinForms on which there was no partial class containing the auto-generated comment.
@Evangelink Thanks for looking into it.
At the very least shouldn't the analysis only be disabled inside the region and not for the entire file? We have very large code files getting completely skipped by the analyzer because of these regions.
It should clearly! That's why I called the current mechanism a hack.
The problem is the time it would take to fix this implementation.
Do you have any idea of how many files are broken because of this behavior? It might help with changing the decision.
@Evangelink I don't think it's very many but since it's a silent failure I'm not sure. Members of our team were scratching our heads for weeks wondering some files were being analyzed but others weren't. Files that weren't being analyzed were known to have analysis results at one point so that made it more confusing.
Is it possible for another analyzer to flag when this analyzer is enabled? e.g. S9999 Analysis disabled for current file. This would make users aware of the problem and give them a chance to fix the file.
Hi @Evangelink, I have also encountered this issue recently.
Our code base has 'generated by' in the legal license agreement header on every file. This means that all files are ignored. It would be great to have a fix for this so that we can fully adopt sonar-dotnet in our toolchain.
We've hit this problem ourselves #4048
@Evangelink, do you remember what was the affected WinForms project type or file? Was it manually written, or generated by some designer?
Thank you
I don't have all details fresh in mind and also it's something that was implemented before me but from what I recall we had some paying customers with old codebases from a time before generated code was put into partial classes. This generated code was raising lots of issues that the users didn't wanted to fix, my investigation concluded that when the hack was put in place there wasn't much way to allow this specific exclusion (roslyn and sonarqube APIs weren't advanced enough).
The biggest problem was that we had only one unit test which wasn't really representative of the real cases. I would recommend to either:
ask the support/consulting team to reach out to customers with old .net codebases to ask if they have cases of these regions to have some kind of estimate of the impact. It might be a bit tricky to have the answer.
discuss with PM and support/consulting about this edge case and agree upon taking the risk to have a breaking change (will need to be introduced in a major version of the analyzer), knowing that you might have some complaints but at least you will have real life scenarios to work with.
I have the feeling that if there are complaints that will most likely be because of all the new (true positive) files analyzed rather than false positives.
Note that when I did the investigation (short one), I could not find any easy (unexpensive) way to check for the region in the reporting helper.
The logic in GeneratedCodeRecognizer should stay the same. Only CSharpGeneratedCodeRecognizer will be modified.
(note for us: I didn't validate on peach, it wouldn't be very easy to do and our test suite is good enough for this scenario)
@andrei-epure-sonarsource it's not obvious from the title or comments of this ticket that the detection of generated regions has been dropped completely. It would be useful to make it a bit more explicit e.g. by updating the title or adding some text in the original comment.
Thanks @duncanp-sonar . I've updated the release notes, as well.
Most helpful comment
I don't have all details fresh in mind and also it's something that was implemented before me but from what I recall we had some paying customers with old codebases from a time before generated code was put into partial classes. This generated code was raising lots of issues that the users didn't wanted to fix, my investigation concluded that when the hack was put in place there wasn't much way to allow this specific exclusion (roslyn and sonarqube APIs weren't advanced enough).
The biggest problem was that we had only one unit test which wasn't really representative of the real cases. I would recommend to either:
ask the support/consulting team to reach out to customers with old .net codebases to ask if they have cases of these regions to have some kind of estimate of the impact. It might be a bit tricky to have the answer.
discuss with PM and support/consulting about this edge case and agree upon taking the risk to have a breaking change (will need to be introduced in a major version of the analyzer), knowing that you might have some complaints but at least you will have real life scenarios to work with.
I have the feeling that if there are complaints that will most likely be because of all the new (true positive) files analyzed rather than false positives.
Note that when I did the investigation (short one), I could not find any easy (unexpensive) way to check for the region in the reporting helper.