Create rule S6354: Testable date/time provider should be used #285
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
andrei-epure-sonarsource
changed the title
andrei-epure-sonarsource
changed the title
andrei-epure-sonarsource
changed the title
andrei-epure-sonarsource
changed the title
rules/S6354/apex/rule.adoc
Outdated
| @@ -0,0 +1,20 @@ | |||
| FIXME: add a description | |||
There was a problem hiding this comment.
Is this intentional to cover apex, go, java, kotlin, PHP, python, ruby, scala, and swift ? Do you plan to add rule descriptions for all these languages?
There was a problem hiding this comment.
AFAIK whenever we create a rule, we should add all languages for which it applies. This way everyone can look at which rules are already specified for a particular language.
The downside of this is needing to create rule descriptions to all languages at once.
I will try to find a generic description, however I won't be able to create code snippets for all.
There was a problem hiding this comment.
I don't think you need to add all languages the rule applies to from the first PR. You can always extend the existing rule later.
Merging this PR, in addition to the useful rspec for .Net languages, will bring multiple unfinished rspecs to master. These unfinished rspecs will have no value, so they will dilute the valuable rules in this PR with irrelevant code.
There was a problem hiding this comment.
I agree with you and if we consider how JIRA was used :
- you had an RSPEC which was applicable to N languages.
- When you wanted to add new rules for a language, you would look into the existing, already specified rules applicable to language X.
- You would then create a JIRA sub-task for language X where you would customize the rule - usually with the
NoncompliantandCompliantcases.
So I added all the tags to be able to keep the same workflow as before - I didn't know of a different option.
I can remove the files from the repo to avoid noise, however I need to be able to search for all the rules which are not_covered but are applicable to language X. I imagine I can do this via github labels, but this is frail and not reflected in the versioning system. Also, it is not searchable locally (which is one of the benefits of having the rspec as github repo).
Do we have any option to keep the same workflow? We'd need another piece of metadata to allow to search which rules are applicable to language X (but not yet implemented, nor specified for language X).
LE: I guess just keeping the github labels is good enough for now.
Later LE: But wait - when this PR gets merged, we lose this information.
There was a problem hiding this comment.
I was not aware of a workflow for searching for "applicable but not specified / underspecified for a language" rules. In CFamily bubble we keep a backlog of tickets with rule ideas that wait for a specification sprint.
If you think rspec should be able to help with this use case, then you need some kind of a marker (a metadata field?) that designates incomplete rspecs. I believe this issue needs a deeper thought and an explicit policy.
AFAIK, right now not everyone would preallocate a rspec for every language it is applicable to when specifying a rule for one particular language. Hence, your search for "applicable but underspecified" rules will be incomplete.
There was a problem hiding this comment.
There was a problem hiding this comment.
The conclusion was that the Languages Team doesn't need this usecase.
I will keep only C# and VB .NET
rules/S6354/vbnet/rule.adoc
Outdated
| Public Function UtcNow() As Date | ||
| End Function | ||
|
|
||
| Public Function SetTimeForCurrentThread(ByVal time As Func(Of Date)) As IDisposable |
There was a problem hiding this comment.
ByVal is an optional keyword that is advised not to use.
We've discussed and in codebases where this approach was not adopted from the beginning, there may be reluctancy to change it and it may be considered as noise.
andrei-epure-sonarsource
force-pushed
the
rule/add-RSPEC-S6354
branch
from
2d610ed
to
dcac2e1
Compare
|
should not be merged before SonarSource/sonar-dotnet#4207 |
|
SonarQube Quality Gate for 'rspec-frontend'
|
|
SonarQube Quality Gate for 'rspec-tools'
|
You can preview this rule here (updated a few minutes after each push).
This rule has been suggested by Corniel on SonarSource/sonar-dotnet#4207