Skip to content

Conversation

@martin-strecker-sonarsource
Copy link
Contributor

Follow up on #9298 and others

The PR replaces all simple cases of calls to

[Obsolete("Use another overload of ReportIssue, without calling Diagnostic.Create")]
public void ReportIssue(Diagnostic diagnostic) =>
ReportIssueCore(diagnostic);

Copy link
Contributor

@pavel-mikula-sonarsource pavel-mikula-sonarsource left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

if (typeSyntax != null && typeSyntax.IsKnownType(KnownType.System_Collections_Generic_List_T, context.SemanticModel))
{
context.ReportIssue(Diagnostic.Create(Rule, typeSyntax.GetLocation(), messageArgs: memberType));
context.ReportIssue(Rule, typeSyntax, messageArgs: memberType);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can go away

Suggested change
context.ReportIssue(Rule, typeSyntax, messageArgs: memberType);
context.ReportIssue(Rule, typeSyntax, memberType);

Comment on lines 59 to 60
c.ReportIssue(Rule, forNode.Condition,
string.Format(CultureInfo.InvariantCulture, MessageFormatNotEmpty, conditionVariables, incrementedVariables));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should fit in a single line. Same below in the else.

Comment on lines 78 to 81
analysisContext.ReportIssue(Rule,
identifierName,
declaration.Identifier.ValueText,
string.Concat(explicitInterfaceSpecifier.Name, ".", identifierName.ValueText));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whitespace: The Rule should be on the next line. Or everything should be on a single line if it fits into 200 chars

forEachStatementSyntax.Expression.GetLocation(),
string.Format(SelectMessageFormat, forEachStatementSyntax.Identifier.ValueText, accessedProperties.Single().Key.Name));
c.ReportIssue(diagnostic);
c.ReportIssue(Rule,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, Rule on the next line, or all in a single line

Rule,
candidate.Locations.FirstOrDefault(),
messageArgs: candidate.Name));
messageArgs: candidate.Name);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can go away. And as you'll open the file, the invocation can be a single line

Suggested change
messageArgs: candidate.Name);
candidate.Name);

private static void Report(SonarSyntaxNodeReportingContext c, VariableDeclaratorSyntax declaratorSyntax) =>
c.ReportIssue(Diagnostic.Create(Rule,
declaratorSyntax.Identifier.GetLocation(),
c.ReportIssue(Rule,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also here

&& IsConstantEmptyString(firstArgument.Expression, c.SemanticModel))
{
c.ReportIssue(Diagnostic.Create(rule, invocationExpression.GetLocation(), MessageFormat));
c.ReportIssue(rule, invocationExpression.GetLocation(), MessageFormat);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
c.ReportIssue(rule, invocationExpression.GetLocation(), MessageFormat);
c.ReportIssue(rule, invocationExpression, MessageFormat);

action,
exportedType.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat),
attributeTargetType.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat)));
c.ReportIssue(Rule,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rule on the next line

@@ -54,10 +54,10 @@ public override void VisitIdentifierName(IdentifierNameSyntax node)
{
if (IsImplicitReturnValue(node))
{
context.ReportIssue(Diagnostic.Create(Rule, node.GetLocation(),
context.ReportIssue(Rule, node,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the formatting on this tone. Single line should do the trick . Otherwise, Rule and node would need separate lines, while the ternary should be on a single line

@@ -107,8 +107,8 @@ private bool CheckExpression(SonarSyntaxNodeReportingContext context, Expression
if (matchCount >= MinimumSeriesLength)
{
var nextStatementCount = matchCount - 1;
context.ReportIssue(Diagnostic.Create(rule, executableStatement.GetLocation(),
nextStatementCount, expression.ToString(), nextStatementCount == 1 ? string.Empty : "s"));
context.ReportIssue(rule, executableStatement,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 4, 2024

Quality Gate Passed Quality Gate passed for 'Sonar .NET Java Plugin'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 4, 2024

@costin-zaharia-sonarsource costin-zaharia-sonarsource removed their assignment Jun 5, 2024
@martin-strecker-sonarsource
Copy link
Contributor Author

martin-strecker-sonarsource commented Jun 5, 2024

Peach validation: All new lost/found issues are on rules changed yesterday as well and peach validation is done in that rules. All other rules are unchanged. The PR with changes are
#9384
#9383
#9385
#9381

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants