Skip to content

Commit

Permalink
Improve S6678: Change primary and secondary locations (#8836)
Browse files Browse the repository at this point in the history
  • Loading branch information
@zsolt-kolbay-sonarsource
zsolt-kolbay-sonarsource committed Feb 29, 2024
1 parent 7ae6c9a commit 88b80ab
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 16 deletions.
2 changes: 1 addition & 1 deletion ...ers/its/expected/ManuallyAddedNoncompliantIssues.CS/S6678-IntentionalFindings-net8.0.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"Id": "S6678",
"Message": "Use PascalCase for named placeholders.",
"Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/ManuallyAddedNoncompliantIssues.CS/IntentionalFindings/S6668.cs#L9",
"Location": "Line 9 Position 43-52"
"Location": "Line 9 Position 32-55"
}
]
}
5 changes: 2 additions & 3 deletions ...zers/src/SonarAnalyzer.CSharp/Rules/MessageTemplates/UsePascalCaseForNamedPlaceHolders.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,8 @@ public void Execute(SonarSyntaxNodeReportingContext context, InvocationExpressio
var nonPascalCasePlaceholders = placeholders.Where(x => char.IsLower(x.Name[0])).ToArray();
if (nonPascalCasePlaceholders.Length > 0)
{
var primaryLocation = GetLocation(nonPascalCasePlaceholders[0]);
var secondaryLocations = nonPascalCasePlaceholders.Skip(1).Select(GetLocation);
context.ReportIssue(Diagnostic.Create(Rule, primaryLocation, secondaryLocations));
var secondaryLocations = nonPascalCasePlaceholders.Select(GetLocation);
context.ReportIssue(Diagnostic.Create(Rule, templateArgument.GetLocation(), secondaryLocations));
}

Location GetLocation(Placeholder placeholder) =>
Expand Down
3 changes: 3 additions & 0 deletions analyzers/tests/SonarAnalyzer.Test/Rules/UsePascalCaseForNamedPlaceHoldersTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ public void Method(ILogger logger, int arg)
{
logger.{{methodName}}("Arg: {Arg}", arg); // Compliant
logger.{{methodName}}("Arg: {arg}", arg); // Noncompliant
// Secondary @-1
}
}
""").Verify();
Expand All @@ -79,6 +80,7 @@ public void Method(ILogger logger, int arg)
{
logger.{{methodName}}("Arg: {Arg}", arg); // Compliant
logger.{{methodName}}("Arg: {arg}", arg); // Noncompliant
// Secondary @-1
}
}
""").Verify();
Expand All @@ -102,6 +104,7 @@ public void Method(ILogger iLogger, Logger logger, MyLogger myLogger, int arg)
{
logger.{{methodName}}("Arg: {Arg}", arg); // Compliant
logger.{{methodName}}("Arg: {arg}", arg); // Noncompliant
// Secondary @-1
}
}
public class MyLogger : Logger { }
Expand Down
38 changes: 26 additions & 12 deletions analyzers/tests/SonarAnalyzer.Test/TestCases/UsePascalCaseForNamedPlaceHolders.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,32 @@ public void Basics(ILogger logger, int arg)
logger.BeginScope("scope");
logger.BeginScope("{arg}", arg);

logger.LogInformation("Arg: {arg}", arg); // Noncompliant {{Use PascalCase for named placeholders.}}
// ^^^
logger.LogInformation("Arg: {Arg} {arg}", arg, arg); // Noncompliant
// ^^^
logger.LogInformation("Arg: {arg} {Arg}", arg, arg); // Noncompliant
// ^^^
logger.LogInformation("Arg: {arg} {Arg}", arg, arg); // Noncompliant
// ^^^
logger.LogInformation("Arg: {arg} {arg}", arg, arg);
// ^^^
logger.LogInformation("Arg: {arg}", arg);
// ^^^^^^^^^^^^ {{Use PascalCase for named placeholders.}}
// ^^^ Secondary @-1

logger.LogInformation("Arg: {Arg} {arg}", arg, arg);
// ^^^^^^^^^^^^^^^^^^
// ^^^ Secondary @-1

logger.LogInformation("Arg: {arg} {Arg}", arg, arg);
// ^^^^^^^^^^^^^^^^^^
// ^^^ Secondary @-1

logger.LogInformation("Arg: {arg} {arg}", arg, arg);
// ^^^^^^^^^^^^^^^^^^
// ^^^ Secondary @-1
// ^^^ Secondary @-2

logger.LogInformation(@"
Arg: {arg}
{arg}", arg, arg);
// ^^^ @-1
// ^^^ Secondary @-1
// Noncompliant @-3
// Secondary @-3
// Secondary @-3

LoggerExtensions.LogInformation(logger, "Arg: {arg}", arg); // Noncompliant
// Secondary @-1

logger.LogInformation("Arg: {Argumentvalue}", arg); // FN - should be {ArgumentValue}, but the analyzer doesn't use any kind of word dictionary, it only checks the first character
}
Expand All @@ -46,21 +54,27 @@ public void NamedArguments(ILogger logger, int arg)
logger.LogInformation(args: new object[] { arg }, message: "Arg: {Arg}"); // Compliant
logger.LogInformation(message: "Arg: {Arg}", args: new object[] { arg }); // Compliant
logger.LogInformation(args: new object[] { arg }, message: "Arg: {arg}"); // Noncompliant
// Secondary @-1
logger.LogInformation(message: "Arg: {arg}", args: new object[] { arg }); // Noncompliant
// Secondary @-1
}

public void IncorrectPlaceholderFormat(ILogger logger, int arg)
{
logger.LogInformation("Arg: {@arg}", arg); // Noncompliant
// Secondary @-1
logger.LogInformation("Arg: {&arg}", arg);
logger.LogInformation("Arg: {arg,23}", arg); // Noncompliant
// Secondary @-1
logger.LogInformation("Arg: {arg,arg}", arg);
}

public void ClassImplementsILogger(CustomLogger logger, int arg)
{
logger.LogCritical("Arg: {arg}", arg); // Noncompliant
// Secondary @-1
logger.LogInformation("Arg: {arg}", arg); // Noncompliant
// Secondary @-1
}

public void ClassDoesNotImplementILogger(NotILogger notILogger, int arg)
Expand Down

0 comments on commit 88b80ab

Please sign in to comment.