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.

Sign up for GitHub

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

support gcc warnings without rule id #1708

Merged
merged 3 commits into from
Apr 25, 2019
Merged

support gcc warnings without rule id #1708

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
694ac8d
fix gcc warnings without rule id
stalb Apr 16, 2019
c41e2f6
add comments and change rule key for gcc warnings without activation
stalb Apr 24, 2019
ea11d5b
add test
stalb Apr 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion cxx-sensors/src/main/java/org/sonar/cxx/sensors/compiler/gcc/CxxCompilerGccSensor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,12 @@ public class CxxCompilerGccSensor extends CxxCompilerSensor {
public static final String REPORT_REGEX_DEF = "gcc.regex";
public static final String REPORT_CHARSET_DEF = "gcc.charset";
public static final String DEFAULT_CHARSET_DEF = "UTF-8";
/**
* Default id used for gcc warnings not associated with any activation switch.
*/
public static final String DEFAULT_ID = "default";
public static final String DEFAULT_REGEX_DEF
= "(?<file>.*):(?<line>[0-9]+):[0-9]+:\\x20warning:\\x20(?<message>.*)\\x20\\[(?<id>.*)\\]";
= "(?<file>.*):(?<line>[0-9]+):[0-9]+:\\x20warning:\\x20(?<message>.*?)(\\x20\\[(?<id>.*)\\])?\\s*$";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are all messages always in one line?

Maybe regex would be better (with id | without id)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my understanding all the warnings look either like:

  • main.c:11:6: warning: unused variable 'i' [-Wunused-variable] where the id is the corresponding switch param (-Wunused-variable) or
  • main.c:7:1: warning: no semicolon at end of struct or union where the id part is missing.

The regex match both of them.
By default warning messages are on one line, The user can force warning messages to be on several lines (if using for example -fmessage-length switch, but then current actual regex won't match either).

A named group can only appears once. So it's not possible to duplicate the beginning of the regex and if I use something like:

  • (?<file>.*):(?<line>[0-9]+):[0-9]+:\\x20warning:\\x20(?<message>.*)(\\x20\\[(?<id>.*)\\])? or
  • (?<file>.*):(?<line>[0-9]+):[0-9]+:\\x20warning:\\x20(?<message>.*)((\\x20\\[(?<id>.*)\\])|(?!(\\x20\\[.*\\]))) (matching id or not matching id)

The id is not recognized anymore.

I solved the problem using the greedy (?) and non-greedy (*?) operators and forcing to match the last part of the line.


public CxxCompilerGccSensor(CxxLanguage language) {
super(language, REPORT_PATH_KEY, CxxCompilerGccRuleRepository.getRepositoryKey(language));
Expand Down Expand Up @@ -70,6 +74,12 @@ protected CxxMetricsFactory.Key getMetricKey() {

@Override
protected String alignId(String id) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add short description why we need this special case.

/* Some gcc warnings are not associated to any activation switch and don't have a matching id.
* In these cases a default id is used.
*/
if (id == null || "".equals(id)) {
id = DEFAULT_ID;
}
return id.replaceAll("=$", "");
}

Expand Down
2 changes: 1 addition & 1 deletion cxx-sensors/src/main/resources/compiler-gcc.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ Follow these steps to make your custom Custom rules available in SonarQube:
<severity>MAJOR</severity>
</rule>
<rule>
<key>enabled by default</key>
<key>default</key>
<name>Default compiler warnings</name>
<description>
Default compiler warnings.
Expand Down
22 changes: 22 additions & 0 deletions cxx-sensors/src/test/java/org/sonar/cxx/sensors/compiler/gcc/CxxCompilerGccSensorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
*/
package org.sonar.cxx.sensors.compiler.gcc;

import java.util.ArrayList;
import static org.assertj.core.api.Assertions.assertThat;
import org.assertj.core.api.SoftAssertions;
import org.junit.Before;
Expand All @@ -28,6 +29,7 @@
import org.sonar.api.batch.fs.internal.TestInputFileBuilder;
import org.sonar.api.batch.sensor.internal.DefaultSensorDescriptor;
import org.sonar.api.batch.sensor.internal.SensorContextTester;
import org.sonar.api.batch.sensor.issue.Issue;
import org.sonar.api.config.internal.MapSettings;
import org.sonar.cxx.CxxLanguage;
import org.sonar.cxx.sensors.compiler.CxxCompilerSensor;
Expand Down Expand Up @@ -76,4 +78,24 @@ public void shouldReportCorrectGccViolations() {
assertThat(context.allIssues()).hasSize(4);
}

@Test
public void shouldReportCorrectGccViolationsWithOrWithoutIds() {
SensorContextTester context = SensorContextTester.create(fs.baseDir());

settings.setProperty(language.getPluginProperty(CxxCompilerGccSensor.REPORT_PATH_KEY), "compiler-reports/build-warning-without-id.gcclog");
context.setSettings(settings);

context.fileSystem().add(TestInputFileBuilder.create("ProjectKey", "main.c")
.setLanguage("c").initMetadata("asd\nasdas\nasda\n").build());

CxxCompilerSensor sensor = new CxxCompilerGccSensor(language);
sensor.execute(context);
assertThat(context.allIssues()).hasSize(2);
ArrayList<Issue> issuesList = new ArrayList<>(context.allIssues());
// warning without activation switch (no id) should be mapped to the "default" rule
assertThat(issuesList.get(0).ruleKey().rule()).isEqualTo("default");
// warning with activation switch should be mapped to the matching rule
assertThat(issuesList.get(1).ruleKey().rule()).isEqualTo("-Wunused-variable");
}

}
7 changes: 7 additions & 0 deletions ...es/org/sonar/cxx/sensors/reports-project/compiler-reports/build-warning-without-id.gcclog
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
main.c:7:1: warning: no semicolon at end of struct or union
} my_struct;
^
main.c: In function 'main':
main.c:11:6: warning: unused variable 'i' [-Wunused-variable]
int i;
^