Skip to content

Commit f88f353

Browse files
committed
parse --xml output instead of text output - fixes #9 / some cleanups
1 parent 0cbeb32 commit f88f353

File tree

7 files changed

+132
-83
lines changed

7 files changed

+132
-83
lines changed

README.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,14 @@ See
2121
2. Install the [cppcheck plugin][cppcheck_plugin] into CLion.
2222
3. Configure the plugin with the **absolute** path to the `cppcheck` executable into the `cppcheck path` option.
2323
1. Windows
24-
1. File | Settings | cppcheck configuration
24+
1. File | Settings | Cppcheck configuration
2525
2. Usually the path is `C:\Program Files (x86)\Cppcheck\cppcheck.exe`
2626
2. macOS:
27-
1. CLion | Preferences | cppcheck configuration
27+
1. CLion | Preferences | Cppcheck configuration
2828
2. In a terminal run `which cppcheck` to find the path to `cppcheck`. If you installed it with
2929
[Homebrew](https://brew.sh/), the path will be `/usr/local/bin/cppcheck`.
3030
3. Linux
31-
1. File | Settings | cppcheck configuration
31+
1. File | Settings | Cppcheck configuration
3232
2. In a terminal run `which cppcheck` to find the path to `cppcheck`. If you installed it with your
3333
system's package manager, it is probably located at `/usr/bin/cppcheck`.
3434

@@ -57,6 +57,8 @@ Deployment.
5757

5858
### 1.6.0 - XXXX-XX-XX
5959

60+
Parse `--xml` output instead of text output. (Contribution by @firewave)
61+
6062
### 1.5.1 - 2020-11-12
6163

6264
Improved reporting of execution failures. (Contribution by @firewave)

resources/META-INF/plugin.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
]]></description>
6262

6363
<change-notes><![CDATA[
64+
Parse --xml output instead of text output. (Contribution by @firewave)<br/>
6465
]]>
6566
</change-notes>
6667

src/com/github/johnthagen/cppcheck/Configuration.java

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ public class Configuration implements Configurable {
1818
"Note: C++ projects should leave --language=c++ appended to the Cppcheck options to avoid some " +
1919
"false positives in header files due to the fact that Cppcheck implicitly defaults to " +
2020
"setting --language to \"c\" for .h files.\n\n" +
21-
"You should not include any --template={} in the options.\n\n" +
2221
"Due to how the Cppcheck plugin executes on one file at a time, `--enable=unusedFunction`" +
2322
"is not supported.";
2423
private static final String CPPCHECK_MISRA_NOTE =
@@ -55,13 +54,13 @@ public String getHelpTopic() {
5554
@Nullable
5655
@Override
5756
public JComponent createComponent() {
58-
JPanel jPanel = new JPanel();
57+
final JPanel jPanel = new JPanel();
5958

60-
VerticalLayout verticalLayout = new VerticalLayout(1, 2);
59+
final VerticalLayout verticalLayout = new VerticalLayout(1, 2);
6160
jPanel.setLayout(verticalLayout);
6261

6362
cppcheckFilePicker = new JFilePicker("Cppcheck Path:", "...");
64-
JLabel optionFieldLabel = new JLabel("Cppcheck Options (Default: " + defaultOptions + "):");
63+
final JLabel optionFieldLabel = new JLabel("Cppcheck Options (Default: " + defaultOptions + "):");
6564
cppcheckOptionsField = new JTextField(defaultOptions, 38);
6665
cppcheckMisraFilePicker = new JFilePicker("MISRA Addon JSON:", "...");
6766

@@ -76,11 +75,11 @@ public JComponent createComponent() {
7675
Properties.set(CONFIGURATION_KEY_CPPCHECK_MISRA_PATH, cppcheckMisraFilePicker.getTextField().getText());
7776
}
7877

79-
JTextArea cppcheckNoteArea = new JTextArea(CPPCHECK_NOTE, 2, 80);
78+
final JTextArea cppcheckNoteArea = new JTextArea(CPPCHECK_NOTE, 2, 80);
8079
cppcheckNoteArea.setLineWrap(true);
8180
cppcheckNoteArea.setWrapStyleWord(true);
8281

83-
JTextArea cppcheckMisraNoteArea = new JTextArea(CPPCHECK_MISRA_NOTE, 2, 80);
82+
final JTextArea cppcheckMisraNoteArea = new JTextArea(CPPCHECK_MISRA_NOTE, 2, 80);
8483
cppcheckMisraNoteArea.setLineWrap(true);
8584
cppcheckMisraNoteArea.setWrapStyleWord(true);
8685

@@ -119,13 +118,13 @@ public void apply() {
119118

120119
@Override
121120
public void reset() {
122-
String cppcheckPath = Properties.get(CONFIGURATION_KEY_CPPCHECK_PATH);
121+
final String cppcheckPath = Properties.get(CONFIGURATION_KEY_CPPCHECK_PATH);
123122
cppcheckFilePicker.getTextField().setText(cppcheckPath);
124123

125-
String cppcheckOptions = Properties.get(CONFIGURATION_KEY_CPPCHECK_OPTIONS);
124+
final String cppcheckOptions = Properties.get(CONFIGURATION_KEY_CPPCHECK_OPTIONS);
126125
cppcheckOptionsField.setText(cppcheckOptions);
127126

128-
String cppcheckMisraPath = Properties.get(CONFIGURATION_KEY_CPPCHECK_MISRA_PATH);
127+
final String cppcheckMisraPath = Properties.get(CONFIGURATION_KEY_CPPCHECK_MISRA_PATH);
129128
cppcheckMisraFilePicker.getTextField().setText(cppcheckMisraPath);
130129

131130
modified = false;

src/com/github/johnthagen/cppcheck/CppCheckInspectionImpl.java

Lines changed: 108 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
import com.intellij.execution.configurations.GeneralCommandLine;
88
import com.intellij.execution.process.CapturingProcessHandler;
99
import com.intellij.execution.process.ProcessOutput;
10+
import com.intellij.notification.Notification;
11+
import com.intellij.notification.NotificationType;
12+
import com.intellij.notification.Notifications;
1013
import com.intellij.openapi.editor.Document;
1114
import com.intellij.openapi.progress.EmptyProgressIndicator;
1215
import com.intellij.openapi.progress.ProcessCanceledException;
@@ -17,13 +20,20 @@
1720
import com.intellij.util.DocumentUtil;
1821
import com.intellij.util.execution.ParametersListUtil;
1922
import org.jetbrains.annotations.NotNull;
20-
21-
import java.nio.file.Paths;
23+
import org.w3c.dom.NamedNodeMap;
24+
import org.w3c.dom.Node;
25+
import org.w3c.dom.NodeList;
26+
import org.xml.sax.InputSource;
27+
import org.xml.sax.SAXException;
28+
29+
import javax.xml.parsers.DocumentBuilder;
30+
import javax.xml.parsers.DocumentBuilderFactory;
31+
import javax.xml.parsers.ParserConfigurationException;
32+
import java.io.File;
33+
import java.io.IOException;
34+
import java.io.StringReader;
2235
import java.util.ArrayList;
2336
import java.util.List;
24-
import java.util.Scanner;
25-
import java.util.regex.Matcher;
26-
import java.util.regex.Pattern;
2737

2838
public class CppCheckInspectionImpl {
2939
private static ProblemHighlightType severityToHighlightType(@NotNull final String severity) {
@@ -45,72 +55,118 @@ private static ProblemHighlightType severityToHighlightType(@NotNull final Strin
4555
}
4656
}
4757

58+
// TODO: make configurable
59+
private static final boolean VERBOSE_LOG = false;
4860
private static final String INCONCLUSIVE_TEXT = ":inconclusive";
4961

5062
@NotNull
51-
public static List<ProblemDescriptor> parseOutput(@NotNull PsiFile psiFile,
52-
@NotNull InspectionManager manager,
53-
@NotNull Document document,
54-
@NotNull String cppcheckOutput,
55-
@NotNull String sourceFileName) {
56-
List<ProblemDescriptor> descriptors = new ArrayList<>();
57-
Scanner scanner = new Scanner(cppcheckOutput);
58-
59-
//Notifications.Bus.notify(new Notification("cppcheck",
60-
// "Info",
61-
// psiFile.getVirtualFile().getCanonicalPath() + "\n" +
62-
// cppcheckOutput,
63-
// NotificationType.INFORMATION));
64-
65-
// Example output line:
66-
// [C:\Users\John Hagen\ClionProjects\test\main.cpp:12]: (style) Variable 'a' is not assigned a value.
67-
// [main.cpp:12] -> [main.cpp:14]: (performance) Variable 'a' is reassigned a value before the old one has been used.
68-
// One line:
69-
// [C:\Users\User\AppData\Local\Temp\___1main.cpp:14]: (warning:inconclusive) accessMoved: Access of moved variable 'a'.
70-
// [C:\Users\User\AppData\Local\Temp\___1main.cpp:14]: (style) unreadVariable: Variable 'name' is assigned a value that is never used.
71-
Pattern pattern = Pattern.compile("^\\[(.+?):(\\d+)](?:\\s+->\\s+\\[.+])?:\\s+\\((\\w+:?\\w+)\\)\\s+(.+)");
72-
73-
String line;
74-
while (scanner.hasNext()) {
75-
line = scanner.nextLine();
76-
Matcher matcher = pattern.matcher(line);
77-
78-
if (!matcher.matches()) {
63+
public static List<ProblemDescriptor> parseOutput(@NotNull final PsiFile psiFile,
64+
@NotNull final InspectionManager manager,
65+
@NotNull final Document document,
66+
@NotNull final String cppcheckOutput,
67+
@NotNull final String sourceFileName) throws IOException, SAXException, ParserConfigurationException {
68+
69+
if (VERBOSE_LOG) {
70+
// TODO: provide XML output via a "Show Cppcheck output" action - event log messages are truncated
71+
Notifications.Bus.notify(new Notification("Cppcheck",
72+
"Cppcheck execution output for " + psiFile.getName(),
73+
cppcheckOutput,
74+
NotificationType.INFORMATION));
75+
}
76+
77+
final List<ProblemDescriptor> descriptors = new ArrayList<>();
78+
79+
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
80+
DocumentBuilder db = dbf.newDocumentBuilder();
81+
final org.w3c.dom.Document doc = db.parse(new InputSource(new StringReader(cppcheckOutput)));
82+
83+
final NodeList errors = doc.getElementsByTagName("error");
84+
for (int i = 0; i < errors.getLength(); ++i) {
85+
/*
86+
<error id="accessMoved" severity="warning" msg="Access of moved variable &apos;a&apos;." verbose="Access of moved variable &apos;a&apos;." cwe="672" hash="6576707224072251515" inconclusive="true">
87+
<location file="/mnt/s/clion/example_lite_2/test.cpp" line="14" column="18" info="Access of moved variable &apos;a&apos;."/>
88+
<location file="/mnt/s/clion/example_lite_2/test.cpp" line="13" column="7" info="Calling std::move(a)"/>
89+
<symbol>a</symbol>
90+
</error>
91+
*/
92+
93+
final Node error = errors.item(i);
94+
final NamedNodeMap attributes = error.getAttributes();
95+
96+
final String id = attributes.getNamedItem("id").getNodeValue();
97+
98+
// Skip the "toomanyconfigs" error
99+
/*
100+
<error id="toomanyconfigs" severity="information" msg="Too many #ifdef configurations - cppcheck only checks 1 of 12 configurations. Use --force to check all configurations." verbose="The checking of the file will be interrupted because there are too many #ifdef configurations. Checking of all #ifdef configurations can be forced by --force command line option or from GUI preferences. However that may increase the checking time." cwe="398">
101+
<location file="C:\Users\Name\AppData\Local\Temp\___valueflow.cpp" line="0" column="0"/>
102+
</error>
103+
*/
104+
if (id.equals("toomanyconfigs")) {
105+
continue;
106+
}
107+
108+
// Skip the "missingIncludeSystem" error
109+
/*
110+
<error id="missingIncludeSystem" severity="information" msg="Cppcheck cannot find all the include files (use --check-config for details)" verbose="Cppcheck cannot find all the include files. Cppcheck can check the code without the include files found. But the results will probably be more accurate if all the include files are found. Please check your project&apos;s include directories and add all of them as include directories for Cppcheck. To see what files Cppcheck cannot find use --check-config."/>
111+
*/
112+
if (id.equals("missingIncludeSystem")) {
113+
continue;
114+
}
115+
116+
final String severity = attributes.getNamedItem("severity").getNodeValue();
117+
final String errorMessage = attributes.getNamedItem("msg").getNodeValue();
118+
final Node inconclusiveNode = attributes.getNamedItem("inconclusive");
119+
final boolean inconclusive = inconclusiveNode != null && inconclusiveNode.getNodeValue().equals("true");
120+
121+
Node location = null;
122+
123+
// look for the first "location" child name
124+
final NodeList children = error.getChildNodes();
125+
for (int j = 0; j < children.getLength(); ++j) {
126+
final Node child = children.item(j);
127+
if (child.getNodeName().equals("location")) {
128+
location = child;
129+
break;
130+
}
131+
}
132+
133+
// ignore entries without location e.g. missingIncludeSystem
134+
if (location == null) {
79135
continue;
80136
}
81137

82-
final String fileName = Paths.get(matcher.group(1)).getFileName().toString();
83-
int lineNumber = Integer.parseInt(matcher.group(2), 10);
84-
final String severity = matcher.group(3);
85-
final String errorMessage = matcher.group(4);
138+
final NamedNodeMap locationAttributes = location.getAttributes();
139+
final String fileName = new File(locationAttributes.getNamedItem("file").getNodeValue()).getName();
140+
int lineNumber = Integer.parseInt(locationAttributes.getNamedItem("line").getNodeValue());
141+
final int column = Integer.parseInt(locationAttributes.getNamedItem("column").getNodeValue()); // TODO
86142

87-
// If a .c or .cpp file #include's header files, Cppcheck will also run on the header files and print
88-
// any errors. These errors don't apply to the current .cpp field and should not be drawn. They can
143+
// If a file #include's header files, Cppcheck will also run on the header files and print
144+
// any errors. These errors don't apply to the current file and should not be drawn. They can
89145
// be distinguished by checking the file name.
90-
// Example:
91-
// Checking Test.cpp ...
92-
// [Test.h:2]: (style) Unused variable: x
93-
// [Test.cpp:3]: (style) Unused variable: y
94146
if (!fileName.equals(sourceFileName)) {
95147
continue;
96148
}
97149

98-
// Cppcheck error or parsing error.
150+
// Cppcheck error
99151
if (lineNumber <= 0 || lineNumber > document.getLineCount()) {
152+
Notifications.Bus.notify(new Notification("Cppcheck",
153+
"Cppcheck line number out-of-bounds " + i,
154+
id + " " + severity + " " + inconclusive + " " + errorMessage + " " + fileName + " " + lineNumber + " " + column,
155+
NotificationType.ERROR));
100156
continue;
101157
}
102158

103159
// Document counts lines starting at 0, rather than 1 like in cppcheck.
104160
lineNumber -= 1;
105161

106162
final int lineStartOffset = DocumentUtil.getFirstNonSpaceCharOffset(document, lineNumber);
107-
final int lintEndOffset = document.getLineEndOffset(lineNumber);
163+
final int lineEndOffset = document.getLineEndOffset(lineNumber);
108164

109165
final ProblemDescriptor problemDescriptor = manager.createProblemDescriptor(
110166
psiFile,
111-
TextRange.create(lineStartOffset, lintEndOffset),
112-
"Cppcheck: (" + severity + ") " + errorMessage,
113-
severityToHighlightType(severity.replace(INCONCLUSIVE_TEXT, "")),
167+
TextRange.create(lineStartOffset, lineEndOffset),
168+
"Cppcheck: (" + severity + (inconclusive ? INCONCLUSIVE_TEXT : "") + ") " + id + ": " + errorMessage,
169+
severityToHighlightType(severity),
114170
true);
115171
descriptors.add(problemDescriptor);
116172
}
@@ -120,20 +176,11 @@ public static List<ProblemDescriptor> parseOutput(@NotNull PsiFile psiFile,
120176
private static final int TIMEOUT_MS = 60 * 1000;
121177

122178
public static String executeCommandOnFile(@NotNull final String command,
123-
@NotNull final String options,
124-
@NotNull final String filePath,
125-
final String cppcheckMisraPath) throws CppcheckError, ExecutionException {
126-
127-
if (options.contains("--template")) {
128-
throw new CppcheckError("Cppcheck options contain --template field. " +
129-
"Please remove this, the plugin defines its own.");
130-
}
131-
179+
@NotNull final String options,
180+
@NotNull final String filePath,
181+
final String cppcheckMisraPath) throws CppcheckError, ExecutionException {
132182
final GeneralCommandLine cmd = new GeneralCommandLine()
133183
.withExePath(command)
134-
.withParameters(ParametersListUtil.parse(
135-
"--template=\"[{file}:{line}]: ({severity}{inconclusive:" + INCONCLUSIVE_TEXT +
136-
"}) {id}: {message}\""))
137184
.withParameters(ParametersListUtil.parse(options))
138185
.withParameters(ParametersListUtil.parse(filePath));
139186

@@ -168,7 +215,7 @@ public static String executeCommandOnFile(@NotNull final String command,
168215
// MISRA Mode and something went wrong with the misra addon
169216
throw new CppcheckError("MISRA Bail\n" +
170217
cmd.getCommandLineString() + "\n" +
171-
"stdout: " + output.getStdout() + "\n"+
218+
"stdout: " + output.getStdout() + "\n" +
172219
"stderr: " + output.getStderr());
173220
}
174221
}

src/com/github/johnthagen/cppcheck/CppcheckInspection.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@
1515
import com.intellij.psi.PsiFile;
1616
import org.jetbrains.annotations.NotNull;
1717
import org.jetbrains.annotations.Nullable;
18+
import org.xml.sax.SAXException;
1819

20+
import javax.xml.parsers.ParserConfigurationException;
1921
import java.io.File;
2022
import java.io.IOException;
2123
import java.util.List;
@@ -49,6 +51,7 @@ public ProblemDescriptor[] checkFile(@NotNull PsiFile file,
4951
if (cppcheckMisraPath != null && !cppcheckMisraPath.isEmpty()) {
5052
cppcheckOptions = String.format("%s --addon=%s", cppcheckOptions, cppcheckMisraPath);
5153
}
54+
cppcheckOptions = String.format("%s --xml", cppcheckOptions);
5255

5356
File tempFile = null;
5457
try {
@@ -58,17 +61,14 @@ public ProblemDescriptor[] checkFile(@NotNull PsiFile file,
5861
CppCheckInspectionImpl.executeCommandOnFile(cppcheckPath, prependIncludeDir(cppcheckOptions, vFile),
5962
tempFile.getAbsolutePath(), cppcheckMisraPath);
6063

61-
if (!cppcheckOutput.isEmpty()) {
62-
final List<ProblemDescriptor> descriptors = CppCheckInspectionImpl.parseOutput(file, manager, document, cppcheckOutput,
63-
tempFile.getName());
64-
return descriptors.toArray(new ProblemDescriptor[0]);
65-
}
66-
} catch (ExecutionException | CppcheckError | IOException ex) {
64+
final List<ProblemDescriptor> descriptors = CppCheckInspectionImpl.parseOutput(file, manager, document, cppcheckOutput,
65+
tempFile.getName());
66+
return descriptors.toArray(new ProblemDescriptor[0]);
67+
} catch (ExecutionException | CppcheckError | IOException | SAXException | ParserConfigurationException ex) {
6768
Notifications.Bus.notify(new Notification("Cppcheck",
6869
"Cppcheck execution failed.",
6970
ex.getClass().getSimpleName() + ": " + ex.getMessage(),
7071
NotificationType.ERROR));
71-
ex.printStackTrace();
7272
} finally {
7373
if (tempFile != null) {
7474
FileUtil.delete(tempFile);

src/com/github/johnthagen/cppcheck/JFilePicker.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@ class JFilePicker extends JPanel {
1616

1717
setLayout(new FlowLayout(FlowLayout.CENTER, 5, 5));
1818

19-
JLabel label = new JLabel(textFieldLabel);
19+
final JLabel label = new JLabel(textFieldLabel);
2020

2121
textField = new JTextField(30);
22-
JButton button = new JButton(buttonLabel);
22+
final JButton button = new JButton(buttonLabel);
2323

2424
button.addActionListener(evt -> buttonActionPerformed());
2525

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
<!DOCTYPE html>
22
<html lang="en">
33
<body>
4-
Runs cppcheck against the current file.
4+
Runs Cppcheck against the current file.
55
<!-- tooltip end -->
66
</body>
77
</html>

0 commit comments

Comments
 (0)