SonarSource / sonar-dotnet

Code analyzer for C# and VB.NET projects
https://redirect.sonarsource.com/plugins/csharp.html
GNU Lesser General Public License v3.0
788 stars 227 forks source link

Update S100: Relax the rule restrictions #320

Open fmallet opened 7 years ago

fmallet commented 7 years ago

Source: http://stackoverflow.com/questions/42759665/does-sonarlint-have-the-custom-dictionary-feature-like-the-fxcop

Evangelink commented 7 years ago

No longer needed since fix #577

SierraNL commented 6 years ago

I would still like to see this opened again, since there are more usecases then the two character acronym mentioned.

For instance: In most companies I've worked for, the company name which is an acrynom, is supposed to have capital casing. This was added to a CustomDictionary.xml for fxcop/CA in the past.

If I would want to add SonarQube analysis to a codebase like that, I would have to turn off the rule, or edit every code file in the project, since these acronyms are in every namespace.

There are more reasons to have casing exceptions, and currently there is no nice solution, while there was one in the FXCop / CA era.

andrei-epure-sonarsource commented 5 years ago

I am reopening this as it seems it is something wanted by our community. We may not necessarily fix this by supporting custom dictionaries, but maybe a different approach (like allowing acronym-like portions inside the name). Also, we may decide to close it again (I'm reopening it to bring it back into discussion internally)

andrei-epure-sonarsource commented 4 years ago

Result of an internal discussion:

Let's relax the rule as "method name has to start with an uppercase letter, not be fully uppercase and not contain underscores". There will always be somebody adding longer acronyms, ex: NIIOMTPLABOPARMBETZHELBETRABSBOMONIMONKONOTDTEKHSTROMONT

andrei-epure-sonarsource commented 4 years ago

I am changing the scope of the initial ticket which was from 2017. Such an old ticket should generally be closed as won't fix.

By relaxing the restrictions, we'd be less noisy and our users wouldn't complain that much. Moreover, we don't plan to be a styling tool, so we'd raise on really obvious bad cases.

andrei-epure-sonarsource commented 3 years ago

RSPEC-100

SonarWay Default Severity:Minor Impact:Low Likelihood:Low

Boggin commented 2 years ago

Is there a workaround for this warning?

pavel-mikula-sonarsource commented 2 years ago

@Boggin The rule is not configurable yet. You can only disable it for now.

NelsonBN commented 2 years ago

It would be very interesting to be able to create a dictionary of exceptions. To we can define acronyms that are ignored in the rule.

E.g: 'DTO' -> CustomDTO, API -> APIConfig

HaGGi13 commented 2 years ago

@andrei-epure-sonarsource & @pavel-mikula-sonarsource Is there any change to provide a possibility to define a dictionary per project or in general for the purpose @NelsonBN described. It would be super handy and it helps to avoid false positives of S101 caused by domain specify abbreviations. In my opinion, it's not only S101 related, but S100 as well.

Please keep us posted or leave a short remake if there is any chance to have a solution for this ticket in place.

I really appreciate and thank you in advance.

andrei-epure-sonarsource commented 2 years ago

@HaGGi13 we have no plans to make it very configurable. We tend to avoid the necessity for configuration in our rules and make them as generic as possible (because they also run inside SonarLint, for example).

How do you find the above suggestion:

Let's relax the rule as "method name has to start with an uppercase letter, not be fully uppercase and not contain underscores". There will always be somebody adding longer acronyms, ex: NIIOMTPLABOPARMBETZHELBETRABSBOMONIMONKONOTDTEKHSTROMONT

This would mean that there will be lots of false negatives. However no false positives...

HaGGi13 commented 2 years ago

@andrei-epure-sonarsource it's a proper approach/solution. Maybe it would be worthwhile to implement the relaxed version as an additional or rather separate rule. This way it would be possible to choose between the original and the relaxed rule, whereby none or one is allowed to be active only.

What do you think?