Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update S100: Relax the rule restrictions #320

Open
fmallet opened this issue May 16, 2017 · 12 comments
Open

Update S100: Relax the rule restrictions #320

fmallet opened this issue May 16, 2017 · 12 comments
Labels
Area: C# C# rules related issues. Type: Rule rework Change rule behavior

Comments

@fmallet
Copy link
Contributor

fmallet commented May 16, 2017

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

@valhristov valhristov changed the title Support custom dictionaries in S100 for adding names that will not raise issues Update S100: Support custom dictionaries for adding names that will not raise issues May 22, 2017
@Evangelink Evangelink added this to the 6.3 milestone Jul 13, 2017
@Evangelink Evangelink self-assigned this Jul 20, 2017
@Evangelink
Copy link
Contributor

No longer needed since fix #577

@SierraNL
Copy link

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
Copy link
Contributor

andrei-epure-sonarsource commented Jun 13, 2019

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
Copy link
Contributor

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 andrei-epure-sonarsource removed the Status: On Hold Postponed or waiting for an answer. label Jul 6, 2020
@andrei-epure-sonarsource andrei-epure-sonarsource added this to the 8.10 milestone Jul 6, 2020
@andrei-epure-sonarsource andrei-epure-sonarsource changed the title Update S100: Support custom dictionaries for adding names that will not raise issues Update S100: Relax the rule restrictions Jul 8, 2020
@andrei-epure-sonarsource
Copy link
Contributor

andrei-epure-sonarsource commented Jul 8, 2020

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 andrei-epure-sonarsource removed this from the 8.10 milestone Jul 16, 2020
@andrei-epure-sonarsource
Copy link
Contributor

andrei-epure-sonarsource commented Apr 21, 2021

RSPEC-100

SonarWay
Default Severity:Minor
Impact:Low
Likelihood:Low

@Boggin
Copy link

Boggin commented Dec 1, 2021

Is there a workaround for this warning?

@pavel-mikula-sonarsource
Copy link
Contributor

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

@NelsonBN
Copy link

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
Copy link

HaGGi13 commented May 10, 2022

@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
Copy link
Contributor

@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
Copy link

HaGGi13 commented Jun 30, 2022

@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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: C# C# rules related issues. Type: Rule rework Change rule behavior
Projects
None yet
Development

No branches or pull requests

9 participants