Rule S1134 and Rule S1135: Track uses of 'FIXME' and 'TODO' #2250
Conversation
ghost
assigned 
Evangelink
changed the title
| @@ -346,5 +346,20 @@ public static bool IsLeftSideOfAssignment(this ExpressionSyntax expression) | |||
| topParenthesizedExpression.Parent is AssignmentExpressionSyntax assignment && | |||
| assignment.Left == topParenthesizedExpression; | |||
| } | |||
|
|
|||
| public static bool IsComment(this SyntaxTrivia trivia) | |||
There was a problem hiding this comment.
This code was duplicated by the Fixme rule, the Todo rule and the comment metrics analyzers.
sonaranalyzer-dotnet/src/SonarAnalyzer.Common/Rules/CommentKeywordBase.cs
Show resolved
Hide resolved
| } | ||
| } | ||
|
|
||
| private static bool IsWordAt(string text, int index, int count) |
There was a problem hiding this comment.
This logic was already there before, it was just moved.
| }); | ||
| } | ||
|
|
||
| private static IEnumerable<int> AllIndexesOf(string text, string value, StringComparison comparisonType = StringComparison.OrdinalIgnoreCase) |
There was a problem hiding this comment.
This logic was already there before, it was just moved.
NB: I extracted the comparison type as a parameter with a default value.
|
|
||
| Namespace Tests.TestCases | ||
| ''' <summary> | ||
| ''' TODO: Do something ' Noncompliant |
There was a problem hiding this comment.
I cannot test for the issue message because it contains the word TODO in a comment and so triggers another issue on the same line which looks really odd when reviewing.
|
|
||
| Namespace Tests.TestCases | ||
| ''' <summary> | ||
| ''' FIXME: Do something ' Noncompliant |
There was a problem hiding this comment.
I cannot test for the issue message because it contains the word FIXME in a comment and so triggers another issue on the same line which looks really odd when reviewing.
Evangelink
changed the title
| { | ||
| public abstract class CommentKeywordBase : SonarDiagnosticAnalyzer | ||
| { | ||
| private const string TODO_Keyword = "TODO"; |
There was a problem hiding this comment.
these being constants, you could use ALL_CAPS (mixing ALL_CAPS with PascalCase looks ugly)
| protected const string FIXME_DiagnosticId = "S1134"; | ||
| protected const string FIXME_MessageFormat = "Take the required action to fix the issue indicated by this '" + FIXME_Keyword + "' comment."; | ||
|
|
||
| protected abstract DiagnosticDescriptor TODO_Diagnostic { get; } |
There was a problem hiding this comment.
I'd just name it {Todo,Fixme}Diagnostic (there's no reason to use underscores)
sonaranalyzer-dotnet/src/SonarAnalyzer.VisualBasic/Rules/CommentKeyword.cs
Show resolved
Hide resolved
| Imports System.Linq | ||
| Imports System.Text | ||
|
|
||
| Namespace Tests.TestCases |
There was a problem hiding this comment.
could you add a ClassName FIXME and a variable FIXME (just to make sure)
| Imports System.Linq | ||
| Imports System.Text | ||
|
|
||
| Namespace Tests.TestCases |
There was a problem hiding this comment.
could you add a ClassName TODO and a variable TODO (just to make sure)
Fix #2041 and Fix #2042