Skip to content

Commit

Permalink
CxxSquidConfiguration
Browse files Browse the repository at this point in the history 
- improve performance: search in XML tree with XPath is very slow (close SonarOpenCommunity#2383)
- add better Visual Studio support
  - support apostrophes around arguments
  - support include paths with apostrophes
  - optimize parsing arguments
  • Loading branch information
@guwirth
guwirth committed Aug 3, 2022
1 parent c910f95 commit 51f6124
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 57 deletions.
65 changes: 55 additions & 10 deletions cxx-squid/src/main/java/org/sonar/cxx/config/CxxSquidConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Locale;
Expand Down Expand Up @@ -66,15 +67,45 @@
* With {@code get} and {@code getValues} the information is read out again afterwards. {@code get} returns the first
* found value for key, whereby the search starts on level. {@code getValues} collects all found values over all levels.
* It starts with the given level and further found values are added to the end of the list.
*
* <code>
* CompilationDatabase
* |-- PredefinedMacros
* |-- SonarProjectProperties
* |-- Global
* | |-- Defines
* | | |-- Value
* | | |-- ...
* | |-- IncludeDirectories
* | |-- Value
* | |-- ...
* |-- Units
* |-- File [path=...]
* | |-- Defines
* | | |-- Value
* | | |-- ...
* | |-- IncludeDirectories
* | |-- Value
* | |-- ...
* | -- File [path=...]
* | -- ...
* </code>
*/
public class CxxSquidConfiguration extends SquidConfiguration {

// Root
public static final String ROOT = "CompilationDatabase";

// Levels
public static final String PREDEFINED_MACROS = "PredefinedMacros";
public static final String SONAR_PROJECT_PROPERTIES = "SonarProjectProperties";
public static final String GLOBAL = "Global";
public static final String UNITS = "Units";

// name of 'File' elements
public static final String FILE = "File";
public static final String ATTR_PATH = "path";

// name of 'Value' elements
public static final String VALUE = "Value";

Expand All @@ -95,9 +126,13 @@ public class CxxSquidConfiguration extends SquidConfiguration {
private static final Logger LOG = Loggers.get(CxxSquidConfiguration.class);

private final XPathFactory xFactory = XPathFactory.instance();
private final LinkedList<Element> parentList = new LinkedList<>();
private Document document;
private HashMap<String, Element> index = new HashMap<>();

// defines order to search for key/value pairs: Units => Global => SonarProjectProperties => PredefinedMacros
private final LinkedList<Element> parentList = new LinkedList<>();

// base directory to resolve relative paths
private String baseDir = "";

public CxxSquidConfiguration() {
Expand All @@ -117,25 +152,29 @@ public CxxSquidConfiguration(String baseDir, Charset encoding) {
super(encoding);
this.baseDir = baseDir;

var root = new Element("CompilationDatabase");
var root = new Element(ROOT);
root.setAttribute(new Attribute("version", "1.0"));
document = new Document(root);

var element = new Element(PREDEFINED_MACROS);
root.addContent(element);
index.put(PREDEFINED_MACROS, element);
parentList.addFirst(element);

element = new Element(SONAR_PROJECT_PROPERTIES);
root.addContent(element);
index.put(SONAR_PROJECT_PROPERTIES, element);
parentList.addFirst(element);

element = new Element(GLOBAL);
root.addContent(element);
index.put(GLOBAL, element);
parentList.addFirst(element);

// UNITS must be first one in the list
element = new Element(UNITS);
root.addContent(element);
index.put(UNITS, element);
parentList.addFirst(element);
}

Expand Down Expand Up @@ -516,22 +555,26 @@ private Element getParentElement(@Nullable Element element) {
*/
@CheckForNull
private Element findLevel(String level, @Nullable Element defaultElement) {
Element element = index.getOrDefault(level, null);
if (element != null) {
return element;
}
String xpath;
if (Verifier.checkElementName(level) == null) {
xpath = "/CompilationDatabase/" + level;
xpath = "/" + ROOT + "/" + level;
} else {
// handle special case 'UNITS empty' no need to search in tree
if (isUnitsEmpty()) {
return defaultElement;
}
xpath = "//" + UNITS + "/File[@path='" + unifyPath(level) + "']";
xpath = "/" + ROOT + "/" + UNITS + "/" + FILE + "[@" + ATTR_PATH + "='" + unifyPath(level) + "']";
}
XPathExpression<Element> expr = xFactory.compile(xpath, Filters.element());
List<Element> elements = expr.evaluate(document);
if (!elements.isEmpty()) {
return elements.get(0);
element = expr.evaluateFirst(document);
if (element == null) {
element = defaultElement;
}
return defaultElement;
return element;
}

/**
Expand All @@ -548,10 +591,12 @@ private Element getKey(String level, String key) {
eLevel = new Element(level);
document.getRootElement().addContent(eLevel);
} else {
eLevel = new Element("File");
eLevel.setAttribute(new Attribute("path", unifyPath(level)));
eLevel = new Element(FILE);
level = unifyPath(level);
eLevel.setAttribute(new Attribute(ATTR_PATH, level));
parentList.getFirst().addContent(eLevel);
}
index.put(level, eLevel);
}
Element eKey = eLevel.getChild(key);
if (eKey == null) {
Expand Down
86 changes: 39 additions & 47 deletions cxx-squid/src/main/java/org/sonar/cxx/config/JsonCompilationDatabase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import org.sonar.api.utils.log.Logger;
import org.sonar.api.utils.log.Loggers;

Expand All @@ -55,53 +54,43 @@ private static void addMacro(String keyValue, Map<String, String> defines) {
}
}

private static String removeApostophs(String str) {
if (str.charAt(0) == '"' && str.charAt(str.length() - 1) == '"') {
return str.substring(1, str.length() - 1);
}
return str;
}

private static Path makeRelativeToCwd(Path cwd, String include) {
return cwd.resolve(include).normalize();
return cwd.resolve(removeApostophs(include)).normalize();
}

/**
* Tokenize command line with support for escaping
*/
private static String[] tokenizeCommandLine(String cmdLine) {
var arg = new StringBuilder(256);
var args = new ArrayList<String>();
var escape = false;
char stringOpen = 0;
var sb = new StringBuilder(512);

// Tokenize command line with support for escaping
for (var ch : cmdLine.toCharArray()) {
if (escape) {
escape = false;
sb.append(ch);
} else {
if (stringOpen == 0) {
// String not open
if (ch == '\\') {
escape = true;
} else if (ch == '\'') {
stringOpen = '\'';
} else if (ch == '\"') {
stringOpen = '\"';
} else if ((ch == ' ')
&& (sb.length() > 0)) {
args.add(sb.toString());
sb = new StringBuilder(512);
}
if (ch != ' ') {
sb.append(ch);
}
} else {
// String open
if (ch == '\\') {
escape = true;
} else if (ch == stringOpen) {
stringOpen = 0;
}
var insideStr = false;

for (int i = 0; i < cmdLine.length(); i++) {
var ch = cmdLine.charAt(i);

sb.append(ch);
if (ch == ' ' && !insideStr) { // blanks separate arguments (outside of strings)
if (arg.length() > 0) {
args.add(removeApostophs(arg.toString()));
arg.setLength(0);
}
} else if (ch == '"') { // begin or end of string
insideStr = !insideStr;
arg.append(ch);
} else {
arg.append(ch);
}
}

if (sb.length() > 0) {
args.add(sb.toString());
if (arg.length() > 0) {
args.add(removeApostophs(arg.toString()));
}

return args.toArray(new String[0]);
Expand Down Expand Up @@ -153,23 +142,24 @@ private void parseCommandObject(JsonCompilationDatabaseCommandObject commandObje

// No need to parse command lines if we have needed information
if (!(commandObject.hasDefines() || commandObject.hasIncludes())) {
String cmdLine;
String[] args;

if (commandObject.hasArguments()) {
cmdLine = commandObject.getArguments().stream().collect(Collectors.joining(" "));
args = commandObject.getArguments().toArray(new String[0]);
if (args.length == 1) {
args = tokenizeCommandLine(args[0]);
}
} else if (commandObject.hasCommand()) {
cmdLine = commandObject.getCommand();
args = tokenizeCommandLine(commandObject.getCommand());
} else {
return;
}

String[] args = tokenizeCommandLine(cmdLine);
var next = ArgNext.NONE;

defines = new HashMap<>();
includes = new ArrayList<>();
var iSystem = new ArrayList<Path>();
var iDirAfter = new ArrayList<Path>();
var next = ArgNext.NONE;

for (var arg : args) {
if (arg.startsWith("-D")) {
Expand Down Expand Up @@ -218,13 +208,15 @@ private void parseCommandObject(JsonCompilationDatabaseCommandObject commandObje
}

private void addDefines(String level, Map<String, String> defines) {
defines.forEach((String k, String v) -> squidConfig.add(level, CxxSquidConfiguration.DEFINES, k + " " + v));
defines.forEach((String k, String v) -> {
squidConfig.add(level, CxxSquidConfiguration.DEFINES, k + " " + v);
});
}

private void addIncludes(String level, List<Path> includes) {
for (var include : includes) {
includes.forEach((Path include) -> {
squidConfig.add(level, CxxSquidConfiguration.INCLUDE_DIRECTORIES, include.toString());
}
});
}

private enum ArgNext {
Expand Down
29 changes: 29 additions & 0 deletions cxx-squid/src/test/java/org/sonar/cxx/config/JsonCompilationDatabaseTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@
import java.nio.file.Paths;
import java.util.List;
import static org.assertj.core.api.Assertions.*;
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.Test;
import org.sonar.api.internal.apachecommons.lang.SystemUtils;

class JsonCompilationDatabaseTest {

Expand Down Expand Up @@ -104,6 +106,33 @@ void testCommandSettings() throws Exception {
.contains(unifyPath("/usr/include"));
}

@Test
void testVsCommandSettings() throws Exception {
Assumptions.assumeTrue(SystemUtils.IS_OS_WINDOWS);

var squidConfig = new CxxSquidConfiguration();

var file = new File("src/test/resources/jsondb/compile_commands.json");

var jsonDb = new JsonCompilationDatabase(squidConfig);
jsonDb.parse(file);

var filename = "C:/Sample/Project/source.cpp";

List<String> defines = squidConfig.getValues(filename, CxxSquidConfiguration.DEFINES);
List<String> includes = squidConfig.getValues(filename, CxxSquidConfiguration.INCLUDE_DIRECTORIES);

assertThat(defines)
.hasSize(14)
.contains("GLOBAL_DEFINE 1")
.contains("_DLL 1");

assertThat(includes)
.hasSize(24)
.contains(unifyPath("/usr/include"))
.contains(unifyPath("C:\\Access\\Module\\AcquisitionBuffer\\PublicInterface"));
}

@Test
void testArgumentParser() throws Exception {
var squidConfig = new CxxSquidConfiguration();
Expand Down
6 changes: 6 additions & 0 deletions cxx-squid/src/test/resources/jsondb/compile_commands.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,5 +63,11 @@
"includes": [
"/usr/local/include"
]
},
{
"_comment_": "Visual Studio sample",
"directory": "C:/Sample/Solutions/",
"command": "\"clang++.exe\" -x c++ \"C:/Sample/Project/source.cpp\" -std=c++14 -Wall -fms-compatibility-version=19.10 -Wmicrosoft -Wno-invalid-token-paste -Wno-unknown-pragmas -Wno-unused-value -fsyntax-only \"-D_MT\" \"-D_DLL\" \"-D_WIN64\" \"-D_WINDOWS\" \"-D_DEBUG\" \"-D_USRDLL\" \"-DSTRICT\" \"-DTGTSVR_PROJECT\" \"-D_WIN32_DCOM\" \"-D_CRT_SECURE_NO_DEPRECATE\" \"-DBOOST_BIND_GLOBAL_PLACEHOLDERS\" \"-DBOOST_THREAD_DYN_LINK\" \"-D_DEBUG_FUNCTIONAL_MACHINERY\" -isystem\"C:/Sample/Project\" -isystem\"C:/Program Files (x86)/Microsoft Visual Studio/2019/Enterprise/VC/Tools/MSVC/14.29.30133/include\" -isystem\"C:/Program Files (x86)/Microsoft Visual Studio/2019/Enterprise/VC/Tools/MSVC/14.29.30133/atlmfc/include\" -isystem\"C:/Program Files (x86)/Microsoft Visual Studio/2019/Enterprise/VC/Auxiliary/VS/include\" -isystem\"C:/Program Files (x86)/Windows Kits/10/Include/10.0.19041.0/ucrt\" -isystem\"C:/Program Files (x86)/Windows Kits/10/Include/10.0.19041.0/um\" -isystem\"C:/Program Files (x86)/Windows Kits/10/Include/10.0.19041.0/shared\" -isystem\"C:/Program Files (x86)/Windows Kits/10/Include/10.0.19041.0/winrt\" -isystem\"C:/Program Files (x86)/Windows Kits/10/Include/10.0.19041.0/cppwinrt\" -I\"C:/Development/svn/Line18/Sample/Tool\" -I\"C:/Development/Sample/Tool/ComComponents/Interfaces\" -I\"C:/Sample/Solutions/PropertySheets/../../../Sample/include\" -I\"C:/Sample/Solutions/PropertySheets/../../../Sample/component/include/IDL\" -I\"C:/Sample/Solutions/PropertySheets/../../../Sample/Tool/ComComponents/Include/IDL\" -I\"C:/Sample/Solutions/PropertySheets/../../../Sample/Tool/ComComponents/Interfaces\" -I\"C:/Sample/Solutions/PropertySheets/../../../Sample/AddOns/Mediators/DeviceConfigX/DeviceConfigX.Com/IDL\" -I\"C:/Sample/Solutions/PropertySheets/../../../..\" -I\"C:/Sample/Solutions/PropertySheets/../../../Access\" -I\"C:/Sample/Solutions/PropertySheets/../../../Access/Framework/Include\" -I\"C:/Sample/Solutions/PropertySheets/../../../Access/Framework/Module/Types\" -I\"C:/Sample/Solutions/PropertySheets/../../../Access/Framework/Module/ModuleUtility\" -I\"C:/Sample/Solutions/PropertySheets/../../../Access/Framework/Project/Time/Time/PublicInterface\" -I\"C:/Sample/Solutions/PropertySheets/../../../Access/Module/AcquisitionBuffer/PublicInterface\"",
"file": "C:/Sample/Project/source.cpp"
}
]

0 comments on commit 51f6124

Please sign in to comment.