From 51f612495ce31968c4aba2348b8bf3b3c3dd7456 Mon Sep 17 00:00:00 2001 From: guwirth Date: Wed, 27 Jul 2022 10:58:36 +0200 Subject: [PATCH] CxxSquidConfiguration - improve performance: search in XML tree with XPath is very slow (close #2383) - add better Visual Studio support - support apostrophes around arguments - support include paths with apostrophes - optimize parsing arguments --- .../cxx/config/CxxSquidConfiguration.java | 65 +++++++++++--- .../cxx/config/JsonCompilationDatabase.java | 86 +++++++++---------- .../config/JsonCompilationDatabaseTest.java | 29 +++++++ .../resources/jsondb/compile_commands.json | 6 ++ 4 files changed, 129 insertions(+), 57 deletions(-) diff --git a/cxx-squid/src/main/java/org/sonar/cxx/config/CxxSquidConfiguration.java b/cxx-squid/src/main/java/org/sonar/cxx/config/CxxSquidConfiguration.java index ee3ba22043..aaedef44eb 100644 --- a/cxx-squid/src/main/java/org/sonar/cxx/config/CxxSquidConfiguration.java +++ b/cxx-squid/src/main/java/org/sonar/cxx/config/CxxSquidConfiguration.java @@ -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; @@ -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. + * + * + * CompilationDatabase + * |-- PredefinedMacros + * |-- SonarProjectProperties + * |-- Global + * | |-- Defines + * | | |-- Value + * | | |-- ... + * | |-- IncludeDirectories + * | |-- Value + * | |-- ... + * |-- Units + * |-- File [path=...] + * | |-- Defines + * | | |-- Value + * | | |-- ... + * | |-- IncludeDirectories + * | |-- Value + * | |-- ... + * | -- File [path=...] + * | -- ... + * */ 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"; @@ -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 parentList = new LinkedList<>(); private Document document; + private HashMap index = new HashMap<>(); + // defines order to search for key/value pairs: Units => Global => SonarProjectProperties => PredefinedMacros + private final LinkedList parentList = new LinkedList<>(); + + // base directory to resolve relative paths private String baseDir = ""; public CxxSquidConfiguration() { @@ -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); } @@ -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 expr = xFactory.compile(xpath, Filters.element()); - List elements = expr.evaluate(document); - if (!elements.isEmpty()) { - return elements.get(0); + element = expr.evaluateFirst(document); + if (element == null) { + element = defaultElement; } - return defaultElement; + return element; } /** @@ -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) { diff --git a/cxx-squid/src/main/java/org/sonar/cxx/config/JsonCompilationDatabase.java b/cxx-squid/src/main/java/org/sonar/cxx/config/JsonCompilationDatabase.java index 0a613546b3..b710dba1fb 100644 --- a/cxx-squid/src/main/java/org/sonar/cxx/config/JsonCompilationDatabase.java +++ b/cxx-squid/src/main/java/org/sonar/cxx/config/JsonCompilationDatabase.java @@ -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; @@ -55,53 +54,43 @@ private static void addMacro(String keyValue, Map 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(); - 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]); @@ -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(); var iDirAfter = new ArrayList(); + var next = ArgNext.NONE; for (var arg : args) { if (arg.startsWith("-D")) { @@ -218,13 +208,15 @@ private void parseCommandObject(JsonCompilationDatabaseCommandObject commandObje } private void addDefines(String level, Map 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 includes) { - for (var include : includes) { + includes.forEach((Path include) -> { squidConfig.add(level, CxxSquidConfiguration.INCLUDE_DIRECTORIES, include.toString()); - } + }); } private enum ArgNext { diff --git a/cxx-squid/src/test/java/org/sonar/cxx/config/JsonCompilationDatabaseTest.java b/cxx-squid/src/test/java/org/sonar/cxx/config/JsonCompilationDatabaseTest.java index 9ca30d79c1..3b0dc7d5bc 100644 --- a/cxx-squid/src/test/java/org/sonar/cxx/config/JsonCompilationDatabaseTest.java +++ b/cxx-squid/src/test/java/org/sonar/cxx/config/JsonCompilationDatabaseTest.java @@ -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 { @@ -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 defines = squidConfig.getValues(filename, CxxSquidConfiguration.DEFINES); + List 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(); diff --git a/cxx-squid/src/test/resources/jsondb/compile_commands.json b/cxx-squid/src/test/resources/jsondb/compile_commands.json index 826429b685..936a109c27 100644 --- a/cxx-squid/src/test/resources/jsondb/compile_commands.json +++ b/cxx-squid/src/test/resources/jsondb/compile_commands.json @@ -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" } ]