From 0520923b9d5c3160a601bffd6ced365592351bda Mon Sep 17 00:00:00 2001 From: guwirth Date: Sun, 29 Nov 2015 18:24:22 +0100 Subject: [PATCH] preprocessor refactoring fix a bug with chained elif else preprocessor directives (close #678) add additional unit test improve performance cleanup source code --- .../cxx/preprocessor/CxxPreprocessor.java | 535 ++++++++++-------- .../lexer/CxxLexerWithPreprocessingTest.java | 66 ++- 2 files changed, 332 insertions(+), 269 deletions(-) diff --git a/cxx-squid/src/main/java/org/sonar/cxx/preprocessor/CxxPreprocessor.java b/cxx-squid/src/main/java/org/sonar/cxx/preprocessor/CxxPreprocessor.java index ba8efe9516..ac33742c57 100644 --- a/cxx-squid/src/main/java/org/sonar/cxx/preprocessor/CxxPreprocessor.java +++ b/cxx-squid/src/main/java/org/sonar/cxx/preprocessor/CxxPreprocessor.java @@ -19,39 +19,30 @@ */ package org.sonar.cxx.preprocessor; -import static com.sonar.sslr.api.GenericTokenType.EOF; -import static com.sonar.sslr.api.GenericTokenType.IDENTIFIER; -import static org.apache.commons.io.FilenameUtils.wildcardMatchOnSystem; -import static org.sonar.cxx.api.CppKeyword.IFDEF; -import static org.sonar.cxx.api.CppKeyword.IFNDEF; -import static org.sonar.cxx.api.CppPunctuator.LT; -import static org.sonar.cxx.api.CxxTokenType.NUMBER; -import static org.sonar.cxx.api.CxxTokenType.PREPROCESSOR; -import static org.sonar.cxx.api.CxxTokenType.STRING; -import static org.sonar.cxx.api.CxxTokenType.WS; +import com.google.common.collect.HashMultimap; +import com.google.common.collect.Lists; +import com.google.common.collect.Multimap; import java.io.File; + import java.util.ArrayList; import java.util.Collection; +import java.util.Deque; import java.util.HashSet; import java.util.Iterator; import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.Deque; import org.apache.commons.lang.StringUtils; +import static org.apache.commons.io.FilenameUtils.wildcardMatchOnSystem; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.sonar.cxx.CxxConfiguration; -import org.sonar.cxx.lexer.CxxLexer; -import org.sonar.squidbridge.SquidAstVisitorContext; -import com.google.common.collect.HashMultimap; -import com.google.common.collect.Lists; -import com.google.common.collect.Multimap; import com.sonar.sslr.api.AstNode; +import com.sonar.sslr.api.AstNodeType; import com.sonar.sslr.api.Grammar; import com.sonar.sslr.api.Preprocessor; //@todo: deprecated, see http://javadocs.sonarsource.org/4.5.2/apidocs/deprecated-list.html import com.sonar.sslr.api.PreprocessorAction; //@todo: deprecated, see http://javadocs.sonarsource.org/4.5.2/apidocs/deprecated-list.html @@ -60,37 +51,70 @@ import com.sonar.sslr.api.Trivia; import com.sonar.sslr.impl.Parser; +import org.sonar.cxx.CxxConfiguration; +import org.sonar.cxx.lexer.CxxLexer; +import org.sonar.squidbridge.SquidAstVisitorContext; + +import static com.sonar.sslr.api.GenericTokenType.EOF; +import static com.sonar.sslr.api.GenericTokenType.IDENTIFIER; + +import static org.sonar.cxx.api.CppKeyword.IFDEF; +import static org.sonar.cxx.api.CppKeyword.IFNDEF; +import static org.sonar.cxx.api.CppPunctuator.LT; +import static org.sonar.cxx.api.CxxTokenType.NUMBER; +import static org.sonar.cxx.api.CxxTokenType.PREPROCESSOR; +import static org.sonar.cxx.api.CxxTokenType.STRING; +import static org.sonar.cxx.api.CxxTokenType.WS; + +import static org.sonar.cxx.preprocessor.CppGrammar.defineLine; +import static org.sonar.cxx.preprocessor.CppGrammar.elifLine; +import static org.sonar.cxx.preprocessor.CppGrammar.elseLine; +import static org.sonar.cxx.preprocessor.CppGrammar.endifLine; +import static org.sonar.cxx.preprocessor.CppGrammar.ifdefLine; +import static org.sonar.cxx.preprocessor.CppGrammar.ifLine; +import static org.sonar.cxx.preprocessor.CppGrammar.includeLine; +import static org.sonar.cxx.preprocessor.CppGrammar.undefLine; + public class CxxPreprocessor extends Preprocessor { + private class State { - public boolean skipping; - public int nestedIfdefs; + + public boolean skipPreprocessorDirectives; + public boolean conditionWasTrue; + public int conditionalInclusionCounter; public File includeUnderAnalysis; public State(File includeUnderAnalysis) { - reset(); + this.skipPreprocessorDirectives = false; + this.conditionWasTrue = false; + this.conditionalInclusionCounter = 0; this.includeUnderAnalysis = includeUnderAnalysis; } public final void reset() { - skipping = false; - nestedIfdefs = 0; + skipPreprocessorDirectives = false; + conditionWasTrue = false; + conditionalInclusionCounter = 0; includeUnderAnalysis = null; } } static class MismatchException extends Exception { - private String why; + + private final String why; MismatchException(String why) { this.why = why; } + @Override public String toString() { return why; } } class Macro { + public Macro(String name, List params, List body, boolean variadic) { this.name = name; this.params = params; @@ -98,6 +122,7 @@ public Macro(String name, List params, List body, boolean variadic this.isVariadic = variadic; } + @Override public String toString() { return name + (params == null ? "" : "(" + serialize(params, ", ") + (isVariadic ? "..." : "") + ")") @@ -118,8 +143,8 @@ public boolean checkArgumentsCount(int count) { private static final Logger LOG = LoggerFactory.getLogger("CxxPreprocessor"); private Parser pplineParser = null; - private MapChain macros = new MapChain(); - private Set analysedFiles = new HashSet(); + private final MapChain macros = new MapChain(); + private final Set analysedFiles = new HashSet(); private SourceCodeProvider codeProvider = new SourceCodeProvider(); private SquidAstVisitorContext context; private ExpressionEvaluator ifExprEvaluator; @@ -127,8 +152,9 @@ public boolean checkArgumentsCount(int count) { private CxxConfiguration conf; public static class Include { - private int line; - private String path; + + private final int line; + private final String path; Include(int line, String path) { this.line = line; @@ -137,13 +163,21 @@ public static class Include { @Override public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } Include that = (Include) o; - if (line != that.line) return false; - if (path != null ? !path.equals(that.path) : that.path != null) return false; + if (line != that.line) { + return false; + } + if (path != null ? !path.equals(that.path) : that.path != null) { + return false; + } return true; } @@ -161,12 +195,11 @@ public int getLine() { return line; } } - private Multimap includedFiles = HashMultimap.create(); - private Multimap missingIncludeFiles = HashMultimap.create(); + private final Multimap includedFiles = HashMultimap.create(); + private final Multimap missingIncludeFiles = HashMultimap.create(); - // state which is not shared between files - private State state = new State(null); - private Deque stateStack = new LinkedList(); + private State currentFileState = new State(null); + private final Deque globalStateStack = new LinkedList(); public CxxPreprocessor(SquidAstVisitorContext context) { this(context, new CxxConfiguration()); @@ -176,17 +209,17 @@ public CxxPreprocessor(SquidAstVisitorContext context, CxxConfiguration this(context, conf, new SourceCodeProvider()); } - private void registerMacros(Map standardMacros) { + private void registerMacros(Map standardMacros) { for (Map.Entry entry : standardMacros.entrySet()) { Token bodyToken; try { bodyToken = Token.builder() - .setLine(1) - .setColumn(0) - .setURI(new java.net.URI("")) - .setValueAndOriginalValue(entry.getValue()) - .setType(STRING) - .build(); + .setLine(1) + .setColumn(0) + .setURI(new java.net.URI("")) + .setValueAndOriginalValue(entry.getValue()) + .setType(STRING) + .build(); } catch (java.net.URISyntaxException e) { throw new RuntimeException(e); } @@ -249,11 +282,15 @@ public Collection getMissingIncludeFiles(File file) { private boolean isCFile(String filePath) { for (String pattern : cFilesPatterns) { if (wildcardMatchOnSystem(filePath, pattern)) { - LOG.trace("Parse '{}' as C file, matches '{}' pattern", filePath, pattern); + if (LOG.isTraceEnabled()) { + LOG.trace("Parse '{}' as C file, matches '{}' pattern", filePath, pattern); + } return true; } } - LOG.trace("Parse '{}' as C++ file", filePath); + if (LOG.isTraceEnabled()) { + LOG.trace("Parse '{}' as C++ file", filePath); + } return false; } @@ -279,7 +316,7 @@ public PreprocessorAction process(List tokens) { if (ttype == PREPROCESSOR) { - AstNode lineAst = null; + AstNode lineAst; try { lineAst = pplineParser.parse(token.getValue()).getFirstChild(); } catch (com.sonar.sslr.api.RecognitionException re) { @@ -287,40 +324,39 @@ public PreprocessorAction process(List tokens) { return new PreprocessorAction(1, Lists.newArrayList(Trivia.createSkippedText(token)), new ArrayList()); } - String lineKind = lineAst.getName(); + AstNodeType lineKind = lineAst.getType(); - if ("ifdefLine".equals(lineKind)) { + if (lineKind == ifdefLine) { return handleIfdefLine(lineAst, token, filePath); - } else if ("elseLine".equals(lineKind)) { - return handleElseLine(lineAst, token, filePath); - } else if ("endifLine".equals(lineKind)) { - return handleEndifLine(lineAst, token, filePath); - } else if ("ifLine".equals(lineKind)) { + } else if (lineKind == ifLine) { return handleIfLine(lineAst, token, filePath); - } else if ("elifLine".equals(lineKind)) { + } else if (lineKind == endifLine) { + return handleEndifLine(lineAst, token, filePath); + } else if (lineKind == elseLine) { + return handleElseLine(lineAst, token, filePath); + } else if (lineKind == elifLine) { return handleElIfLine(lineAst, token, filePath); } - if (inSkippingMode()) { + if (currentFileState.skipPreprocessorDirectives) { return new PreprocessorAction(1, Lists.newArrayList(Trivia.createSkippedText(token)), new ArrayList()); } - if ("defineLine".equals(lineKind)) { + if (lineKind == defineLine) { return handleDefineLine(lineAst, token, filePath); - } else if ("includeLine".equals(lineKind)) { + } else if (lineKind == includeLine) { return handleIncludeLine(lineAst, token, filePath); - } else if ("undefLine".equals(lineKind)) { + } else if (lineKind == undefLine) { return handleUndefLine(lineAst, token, filePath); } // Ignore all other preprocessor directives (which are not handled explicitly) // and strip them from the stream - return new PreprocessorAction(1, Lists.newArrayList(Trivia.createSkippedText(token)), new ArrayList()); } if (ttype != EOF) { - if (inSkippingMode()) { + if (currentFileState.skipPreprocessorDirectives) { return new PreprocessorAction(1, Lists.newArrayList(Trivia.createSkippedText(token)), new ArrayList()); } @@ -342,7 +378,7 @@ public void finishedPreprocessing(File file) { analysedFiles.clear(); macros.clearLowPrio(); - state.reset(); + currentFileState.reset(); currentContextFile = null; } @@ -355,107 +391,125 @@ public String valueOf(String macroname) { return result; } - private PreprocessorAction handleIfdefLine(AstNode ast, Token token, String filename) { - if (state.skipping) { - state.nestedIfdefs++; - } - else { - Macro macro = macros.get(getMacroName(ast)); - TokenType tokType = ast.getToken().getType(); - if ((tokType == IFDEF && macro == null) || (tokType == IFNDEF && macro != null)) { - LOG.trace("[{}:{}]: '{}' evaluated to false, skipping tokens that follow", - new Object[] {filename, token.getLine(), token.getValue()}); - state.skipping = true; + PreprocessorAction handleIfLine(AstNode ast, Token token, String filename) { + if (!currentFileState.skipPreprocessorDirectives) { + if (LOG.isTraceEnabled()) { + LOG.trace("[{}:{}]: handling #if line '{}'", + new Object[]{filename, token.getLine(), token.getValue()}); } + try { + currentFileState.skipPreprocessorDirectives = false; + currentFileState.skipPreprocessorDirectives = !ifExprEvaluator.eval(ast.getFirstDescendant(CppGrammar.constantExpression)); + } catch (EvaluationException e) { + LOG.error("[{}:{}]: error evaluating the expression {} assume 'true' ...", + new Object[]{filename, token.getLine(), token.getValue()}); + LOG.error("{}", e); + currentFileState.skipPreprocessorDirectives = false; + } + + if (currentFileState.skipPreprocessorDirectives) { + if (LOG.isTraceEnabled()) { + LOG.trace("[{}:{}]: '{}' evaluated to false, skipping tokens that follow", + new Object[]{filename, token.getLine(), token.getValue()}); + } + } else { + currentFileState.conditionWasTrue = true; + } + } else { + currentFileState.conditionalInclusionCounter++; } return new PreprocessorAction(1, Lists.newArrayList(Trivia.createSkippedText(token)), new ArrayList()); } - PreprocessorAction handleElseLine(AstNode ast, Token token, String filename) { - if (state.nestedIfdefs == 0) { - if (state.skipping) { - LOG.trace("[{}:{}]: #else, returning to non-skipping mode", filename, token.getLine()); - } - else { - LOG.trace("[{}:{}]: skipping tokens inside the #else", filename, token.getLine()); - } + PreprocessorAction handleElIfLine(AstNode ast, Token token, String filename) { + // Handling of an elif line is similar to handling of an if line but doesn't increase the nesting level + if (currentFileState.conditionalInclusionCounter == 0) { + if (currentFileState.skipPreprocessorDirectives && !currentFileState.conditionWasTrue) { //the preceeding clauses had been evaluated to false + try { + if (LOG.isTraceEnabled()) { + LOG.trace("[{}:{}]: handling #elif line '{}'", + new Object[]{filename, token.getLine(), token.getValue()}); + } + currentFileState.skipPreprocessorDirectives = false; + currentFileState.skipPreprocessorDirectives = !ifExprEvaluator.eval(ast.getFirstDescendant(CppGrammar.constantExpression)); + } catch (EvaluationException e) { + LOG.error("[{}:{}]: error evaluating the expression {} assume 'true' ...", + new Object[]{filename, token.getLine(), token.getValue()}); + LOG.error("{}", e); + currentFileState.skipPreprocessorDirectives = false; + } - state.skipping = !state.skipping; + if (currentFileState.skipPreprocessorDirectives) { + if (LOG.isTraceEnabled()) { + LOG.trace("[{}:{}]: '{}' evaluated to false, skipping tokens that follow", + new Object[]{filename, token.getLine(), token.getValue()}); + } + } else { + currentFileState.conditionWasTrue = true; + } + } else { + if (LOG.isTraceEnabled()) { + LOG.trace("[{}:{}]: skipping tokens inside the #elif", filename, token.getLine()); + } + currentFileState.skipPreprocessorDirectives = true; + } } return new PreprocessorAction(1, Lists.newArrayList(Trivia.createSkippedText(token)), new ArrayList()); } - PreprocessorAction handleEndifLine(AstNode ast, Token token, String filename) { - if (state.nestedIfdefs > 0) { - state.nestedIfdefs--; - } - else { - if (state.skipping) { - LOG.trace("[{}:{}]: #endif, returning to non-skipping mode", filename, token.getLine()); + private PreprocessorAction handleIfdefLine(AstNode ast, Token token, String filename) { + if (!currentFileState.skipPreprocessorDirectives) { + Macro macro = macros.get(getMacroName(ast)); + TokenType tokType = ast.getToken().getType(); + if ((tokType == IFDEF && macro == null) || (tokType == IFNDEF && macro != null)) { + if (LOG.isTraceEnabled()) { + LOG.trace("[{}:{}]: '{}' evaluated to false, skipping tokens that follow", + new Object[]{filename, token.getLine(), token.getValue()}); + } + currentFileState.skipPreprocessorDirectives = true; + } + if (!currentFileState.skipPreprocessorDirectives) { + currentFileState.conditionWasTrue = true; } - state.skipping = false; + } else { + currentFileState.conditionalInclusionCounter++; } return new PreprocessorAction(1, Lists.newArrayList(Trivia.createSkippedText(token)), new ArrayList()); } - PreprocessorAction handleIfLine(AstNode ast, Token token, String filename) { - if (state.skipping) { - state.nestedIfdefs++; - } - else { - LOG.trace("[{}:{}]: handling #if line '{}'", - new Object[] {filename, token.getLine(), token.getValue()}); - try { - state.skipping = !ifExprEvaluator.eval(ast.getFirstDescendant(CppGrammar.constantExpression)); - } catch (EvaluationException e) { - LOG.error("[{}:{}]: error evaluating the expression {} assume 'true' ...", - new Object[] {filename, token.getLine(), token.getValue()}); - LOG.error("{}", e); - state.skipping = false; - } - - if (state.skipping) { - LOG.trace("[{}:{}]: '{}' evaluated to false, skipping tokens that follow", - new Object[] {filename, token.getLine(), token.getValue()}); + PreprocessorAction handleElseLine(AstNode ast, Token token, String filename) { + if (currentFileState.conditionalInclusionCounter == 0) { + if (currentFileState.skipPreprocessorDirectives && !currentFileState.conditionWasTrue) { + if (LOG.isTraceEnabled()) { + LOG.trace("[{}:{}]: #else, returning to non-skipping mode", filename, token.getLine()); + } + currentFileState.skipPreprocessorDirectives = false; + currentFileState.conditionWasTrue = true; + } else { + if (LOG.isTraceEnabled()) { + LOG.trace("[{}:{}]: skipping tokens inside the #else", filename, token.getLine()); + } + currentFileState.skipPreprocessorDirectives = true; } } return new PreprocessorAction(1, Lists.newArrayList(Trivia.createSkippedText(token)), new ArrayList()); } - PreprocessorAction handleElIfLine(AstNode ast, Token token, String filename) { - // Handling of an elif line is similar to handling of an if line but - // doesn't increase the nesting level - if (state.nestedIfdefs == 0) { - if (state.skipping) { //the preceeding clauses had been evaluated to false - try { - LOG.trace("[{}:{}]: handling #elif line '{}'", - new Object[] {filename, token.getLine(), token.getValue()}); - - // *this* preprocessor instance is used for evaluation, too. - // It *must not* be in skipping mode while evaluating expressions. - state.skipping = false; - - state.skipping = !ifExprEvaluator.eval(ast.getFirstDescendant(CppGrammar.constantExpression)); - } catch (EvaluationException e) { - LOG.error("[{}:{}]: error evaluating the expression {} assume 'true' ...", - new Object[] {filename, token.getLine(), token.getValue()}); - LOG.error("{}", e); - state.skipping = false; - } - - if (state.skipping) { - LOG.trace("[{}:{}]: '{}' evaluated to false, skipping tokens that follow", - new Object[] {filename, token.getLine(), token.getValue()}); + PreprocessorAction handleEndifLine(AstNode ast, Token token, String filename) { + if (currentFileState.conditionalInclusionCounter > 0) { + currentFileState.conditionalInclusionCounter--; + } else { + if (currentFileState.skipPreprocessorDirectives) { + if (LOG.isTraceEnabled()) { + LOG.trace("[{}:{}]: #endif, returning to non-skipping mode", filename, token.getLine()); } + currentFileState.skipPreprocessorDirectives = false; } - else { - state.skipping = !state.skipping; - LOG.trace("[{}:{}]: skipping tokens inside the #elif", filename, token.getLine()); - } + currentFileState.conditionWasTrue = false; } return new PreprocessorAction(1, Lists.newArrayList(Trivia.createSkippedText(token)), new ArrayList()); @@ -466,7 +520,9 @@ PreprocessorAction handleDefineLine(AstNode ast, Token token, String filename) { Macro macro = parseMacroDefinition(ast); if (macro != null) { - LOG.trace("[{}:{}]: storing macro: '{}'", new Object[] {filename, token.getLine(), macro}); + if (LOG.isTraceEnabled()) { + LOG.trace("[{}:{}]: storing macro: '{}'", new Object[]{filename, token.getLine(), macro}); + } macros.put(macro.name, macro); } @@ -497,30 +553,28 @@ PreprocessorAction handleIncludeLine(AstNode ast, Token token, String filename) } if (includedFile == null) { - if (conf.getMissingIncludeWarningsEnabled()){ + if (conf.getMissingIncludeWarningsEnabled()) { LOG.warn("[" + filename + ":" + token.getLine() + "]: cannot find the sources for '" - + token.getValue() + "'"); + + token.getValue() + "'"); } if (currentFile != null) { missingIncludeFiles.put(currentFile.getPath(), new Include(token.getLine(), token.getValue())); } - } - else if (!analysedFiles.contains(includedFile)) { + } else if (!analysedFiles.contains(includedFile)) { analysedFiles.add(includedFile.getAbsoluteFile()); LOG.debug("[{}:{}]: processing {}, resolved to file '{}'", - new Object[] {filename, token.getLine(), token.getValue(), includedFile.getAbsolutePath()}); + new Object[]{filename, token.getLine(), token.getValue(), includedFile.getAbsolutePath()}); - stateStack.push(state); - state = new State(includedFile); + globalStateStack.push(currentFileState); + currentFileState = new State(includedFile); try { IncludeLexer.create(this).lex(codeProvider.getSourceCode(includedFile)); } finally { - state = stateStack.pop(); + currentFileState = globalStateStack.pop(); } - } - else { - LOG.debug("[{}:{}]: skipping already included file '{}'", new Object[] {filename, token.getLine(), includedFile}); + } else { + LOG.debug("[{}:{}]: skipping already included file '{}'", new Object[]{filename, token.getLine(), includedFile}); } return new PreprocessorAction(1, Lists.newArrayList(Trivia.createSkippedText(token)), new ArrayList()); @@ -542,17 +596,16 @@ PreprocessorAction handleIdentifiersAndKeywords(List tokens, Token curr, PreprocessorAction ppaction = PreprocessorAction.NO_OPERATION; Macro macro = macros.get(curr.getValue()); if (macro != null) { - List replTokens = new LinkedList(); + List replTokens = new LinkedList<>(); int tokensConsumed = 0; if (macro.params == null) { tokensConsumed = 1; - replTokens = new LinkedList(expandMacro(macro.name, serialize(evaluateHashhashOperators(macro.body)))); - } - else { + replTokens = new LinkedList<>(expandMacro(macro.name, serialize(evaluateHashhashOperators(macro.body)))); + } else { int tokensConsumedMatchingArgs = expandFunctionLikeMacro(macro.name, - tokens.subList(1, tokens.size()), - replTokens); + tokens.subList(1, tokens.size()), + replTokens); if (tokensConsumedMatchingArgs > 0) { tokensConsumed = 1 + tokensConsumedMatchingArgs; } @@ -561,9 +614,9 @@ PreprocessorAction handleIdentifiersAndKeywords(List tokens, Token curr, if (tokensConsumed > 0) { // Rescanning to expand function like macros, in case it requires consuming more tokens - List outTokens = new LinkedList(); + List outTokens = new LinkedList<>(); macros.disable(macro.name); - while(!replTokens.isEmpty()) { + while (!replTokens.isEmpty()) { Token c = replTokens.get(0); PreprocessorAction action = PreprocessorAction.NO_OPERATION; if (c.getType() == IDENTIFIER) { @@ -574,15 +627,13 @@ PreprocessorAction handleIdentifiersAndKeywords(List tokens, Token curr, if (action == PreprocessorAction.NO_OPERATION) { replTokens.remove(0); outTokens.add(c); - } - else { + } else { outTokens.addAll(action.getTokensToInject()); int tokensConsumedRescanning = action.getNumberOfConsumedTokens(); if (tokensConsumedRescanning >= replTokens.size()) { tokensConsumed += tokensConsumedRescanning - replTokens.size(); replTokens.clear(); - } - else { + } else { replTokens.subList(0, tokensConsumedRescanning).clear(); } } @@ -592,16 +643,18 @@ PreprocessorAction handleIdentifiersAndKeywords(List tokens, Token curr, replTokens = reallocate(replTokens, curr); - LOG.trace("[{}:{}]: replacing '" + curr.getValue() - + (tokensConsumed == 1 - ? "" - : serialize(tokens.subList(1, tokensConsumed))) + "' -> '" + serialize(replTokens) + "'", + if (LOG.isTraceEnabled()) { + LOG.trace("[{}:{}]: replacing '" + curr.getValue() + + (tokensConsumed == 1 + ? "" + : serialize(tokens.subList(1, tokensConsumed))) + "' -> '" + serialize(replTokens) + "'", filename, curr.getLine()); + } ppaction = new PreprocessorAction( - tokensConsumed, - Lists.newArrayList(Trivia.createSkippedText(tokens.subList(0, tokensConsumed))), - replTokens); + tokensConsumed, + Lists.newArrayList(Trivia.createSkippedText(tokens.subList(0, tokensConsumed))), + replTokens); } } @@ -609,14 +662,13 @@ PreprocessorAction handleIdentifiersAndKeywords(List tokens, Token curr, } public String expandFunctionLikeMacro(String macroName, List restTokens) { - List expansion = new LinkedList(); + List expansion = new LinkedList<>(); expandFunctionLikeMacro(macroName, restTokens, expansion); return serialize(expansion); } private int expandFunctionLikeMacro(String macroName, List restTokens, List expansion) { - List replTokens = null; - List arguments = new ArrayList(); + List arguments = new ArrayList<>(); int tokensConsumedMatchingArgs = matchArguments(restTokens, arguments); Macro macro = macros.get(macroName); @@ -627,14 +679,14 @@ private int expandFunctionLikeMacro(String macroName, List restTokens, Li Token firstToken = vaargs.get(0); arguments = arguments.subList(0, macro.params.size() - 1); arguments.add(Token.builder() - .setLine(firstToken.getLine()) - .setColumn(firstToken.getColumn()) - .setURI(firstToken.getURI()) - .setValueAndOriginalValue(serialize(vaargs, ",")) - .setType(STRING) - .build()); + .setLine(firstToken.getLine()) + .setColumn(firstToken.getColumn()) + .setURI(firstToken.getURI()) + .setValueAndOriginalValue(serialize(vaargs, ",")) + .setType(STRING) + .build()); } - replTokens = replaceParams(macro.body, macro.params, arguments); + List replTokens = replaceParams(macro.body, macro.params, arguments); replTokens = evaluateHashhashOperators(replTokens); expansion.addAll(expandMacro(macro.name, serialize(replTokens))); } @@ -655,10 +707,9 @@ private List expandMacro(String macroName, String macroExpression) { } private List stripEOF(List tokens) { - if (tokens.get(tokens.size() - 1).getType() == EOF){ + if (tokens.get(tokens.size() - 1).getType() == EOF) { return tokens.subList(0, tokens.size() - 1); - } - else{ + } else { return tokens; } } @@ -668,7 +719,7 @@ private String serialize(List tokens) { } private String serialize(List tokens, String spacer) { - List values = new LinkedList(); + List values = new LinkedList<>(); for (Token t : tokens) { values.add(t.getValue()); } @@ -717,18 +768,18 @@ private List matchArgument(List tokens, List arguments) thr Token firstToken = tokens.get(0); Token currToken = firstToken; String curr = currToken.getValue(); - List matchedTokens = new LinkedList(); + List matchedTokens = new LinkedList<>(); while (true) { if (nestingLevel == 0 && (",".equals(curr) || ")".equals(curr))) { if (tokensConsumed > 0) { arguments.add(Token.builder() - .setLine(firstToken.getLine()) - .setColumn(firstToken.getColumn()) - .setURI(firstToken.getURI()) - .setValueAndOriginalValue(serialize(matchedTokens).trim()) - .setType(STRING) - .build()); + .setLine(firstToken.getLine()) + .setColumn(firstToken.getColumn()) + .setURI(firstToken.getURI()) + .setValueAndOriginalValue(serialize(matchedTokens).trim()) + .setType(STRING) + .build()); } return tokens.subList(tokensConsumed, noTokens); } @@ -754,9 +805,9 @@ private List replaceParams(List body, List parameters, List // Replace all parameters by according arguments // "Stringify" the argument if the according parameter is preceded by an # - List newTokens = new ArrayList(); + List newTokens = new ArrayList<>(); if (!body.isEmpty()) { - List defParamValues = new ArrayList(); + List defParamValues = new ArrayList<>(); for (Token t : parameters) { defParamValues.add(t.getValue()); } @@ -834,7 +885,7 @@ private List replaceParams(List body, List parameters, List } private List evaluateHashhashOperators(List tokens) { - List newTokens = new ArrayList(); + List newTokens = new ArrayList<>(); Iterator it = tokens.iterator(); while (it.hasNext()) { @@ -843,13 +894,13 @@ private List evaluateHashhashOperators(List tokens) { Token pred = predConcatToken(newTokens); Token succ = succConcatToken(it); newTokens.add(Token.builder() - .setLine(pred.getLine()) - .setColumn(pred.getColumn()) - .setURI(pred.getURI()) - .setValueAndOriginalValue(pred.getValue() + succ.getValue()) - .setType(pred.getType()) - .setGeneratedCode(true) - .build()); + .setLine(pred.getLine()) + .setColumn(pred.getColumn()) + .setURI(pred.getURI()) + .setValueAndOriginalValue(pred.getValue() + succ.getValue()) + .setType(pred.getType()) + .setGeneratedCode(true) + .build()); } else { newTokens.add(curr); } @@ -862,20 +913,20 @@ private Token predConcatToken(List tokens) { while (!tokens.isEmpty()) { Token last = tokens.remove(tokens.size() - 1); if (last.getType() != WS) { - if ( !tokens.isEmpty() ) { + if (!tokens.isEmpty()) { Token pred = tokens.get(tokens.size() - 1); if (pred.getType() != WS && !pred.hasTrivia()) { // Needed to paste tokens 0 and x back together after #define N(hex) 0x ## hex tokens.remove(tokens.size() - 1); String replacement = pred.getValue() + last.getValue(); last = Token.builder() - .setLine(pred.getLine()) - .setColumn(pred.getColumn()) - .setURI(pred.getURI()) - .setValueAndOriginalValue(replacement) - .setType(pred.getType()) - .setGeneratedCode(true) - .build(); + .setLine(pred.getLine()) + .setColumn(pred.getColumn()) + .setURI(pred.getURI()) + .setValueAndOriginalValue(replacement) + .setType(pred.getType()) + .setGeneratedCode(true) + .build(); } } return last; @@ -944,17 +995,17 @@ private String encloseWithQuotes(String str) { } private List reallocate(List tokens, Token token) { - List reallocated = new LinkedList(); + List reallocated = new LinkedList<>(); int currColumn = token.getColumn(); for (Token t : tokens) { reallocated.add(Token.builder() - .setLine(token.getLine()) - .setColumn(currColumn) - .setURI(token.getURI()) - .setValueAndOriginalValue(t.getValue()) - .setType(t.getType()) - .setGeneratedCode(true) - .build()); + .setLine(token.getLine()) + .setColumn(currColumn) + .setURI(token.getURI()) + .setValueAndOriginalValue(t.getValue()) + .setType(t.getType()) + .setGeneratedCode(true) + .build()); currColumn += t.getValue().length() + 1; } @@ -963,7 +1014,7 @@ private List reallocate(List tokens, Token token) { private Macro parseMacroDefinition(String macroDef) { return parseMacroDefinition(pplineParser.parse(macroDef) - .getFirstDescendant(CppGrammar.defineLine)); + .getFirstDescendant(CppGrammar.defineLine)); } private Macro parseMacroDefinition(AstNode defineLineAst) { @@ -973,34 +1024,34 @@ private Macro parseMacroDefinition(AstNode defineLineAst) { AstNode paramList = ast.getFirstDescendant(CppGrammar.parameterList); List macroParams = paramList == null - ? "objectlikeMacroDefinition".equals(ast.getName()) ? null : new LinkedList() - : getParams(paramList); + ? "objectlikeMacroDefinition".equals(ast.getName()) ? null : new LinkedList() + : getParams(paramList); AstNode vaargs = ast.getFirstDescendant(CppGrammar.variadicparameter); if (vaargs != null) { - AstNode identifier = vaargs.getFirstChild(IDENTIFIER); - macroParams.add(identifier == null - ? Token.builder() - .setLine(vaargs.getToken().getLine()) - .setColumn(vaargs.getToken().getColumn()) - .setURI(vaargs.getToken().getURI()) - .setValueAndOriginalValue("__VA_ARGS__") - .setType(IDENTIFIER) - .setGeneratedCode(true) - .build() - : identifier.getToken()); + AstNode identifier = vaargs.getFirstChild(IDENTIFIER); + macroParams.add(identifier == null + ? Token.builder() + .setLine(vaargs.getToken().getLine()) + .setColumn(vaargs.getToken().getColumn()) + .setURI(vaargs.getToken().getURI()) + .setValueAndOriginalValue("__VA_ARGS__") + .setType(IDENTIFIER) + .setGeneratedCode(true) + .build() + : identifier.getToken()); } AstNode replList = ast.getFirstDescendant(CppGrammar.replacementList); List macroBody = replList == null - ? new LinkedList() - : replList.getTokens().subList(0, replList.getTokens().size() - 1); + ? new LinkedList() + : replList.getTokens().subList(0, replList.getTokens().size() - 1); return new Macro(macroName, macroParams, macroBody, vaargs != null); } private List getParams(AstNode identListAst) { - List params = new ArrayList(); + List params = new ArrayList<>(); if (identListAst != null) { for (AstNode node : identListAst.getChildren(IDENTIFIER)) { params.add(node.getToken()); @@ -1016,10 +1067,10 @@ private File findIncludedFile(AstNode ast, Token token, String currFileName) { boolean quoted = false; AstNode node = ast.getFirstDescendant(CppGrammar.includeBodyQuoted); - if(node != null){ + if (node != null) { includedFileName = stripQuotes(node.getFirstChild().getTokenValue()); quoted = true; - } else if((node = ast.getFirstDescendant(CppGrammar.includeBodyBracketed)) != null) { + } else if ((node = ast.getFirstDescendant(CppGrammar.includeBodyBracketed)) != null) { node = node.getFirstDescendant(LT).getNextSibling(); StringBuilder sb = new StringBuilder(); while (true) { @@ -1032,23 +1083,22 @@ private File findIncludedFile(AstNode ast, Token token, String currFileName) { } includedFileName = sb.toString(); - } else if((node = ast.getFirstDescendant(CppGrammar.includeBodyFreeform)) != null) { + } else if ((node = ast.getFirstDescendant(CppGrammar.includeBodyFreeform)) != null) { // expand and recurse String includeBody = serialize(stripEOF(node.getTokens()), ""); String expandedIncludeBody = serialize(stripEOF(CxxLexer.create(this).lex(includeBody)), ""); boolean parseError = false; AstNode includeBodyAst = null; - try{ + try { includeBodyAst = pplineParser.parse("#include " + expandedIncludeBody); - } - catch(com.sonar.sslr.api.RecognitionException re){ + } catch (com.sonar.sslr.api.RecognitionException re) { parseError = true; } - if(parseError || includeBodyAst.getFirstDescendant(CppGrammar.includeBodyFreeform) != null){ + if (parseError || includeBodyAst.getFirstDescendant(CppGrammar.includeBodyFreeform) != null) { LOG.warn("[{}:{}]: cannot parse included filename: {}'", - new Object[] {currFileName, token.getLine(), expandedIncludeBody}); + new Object[]{currFileName, token.getLine(), expandedIncludeBody}); return null; } @@ -1073,13 +1123,10 @@ private String stripQuotes(String str) { } private File getFileUnderAnalysis() { - if (state.includeUnderAnalysis == null) { + if (currentFileState.includeUnderAnalysis == null) { return context.getFile(); } - return state.includeUnderAnalysis; + return currentFileState.includeUnderAnalysis; } - private boolean inSkippingMode() { - return state.skipping; - } } diff --git a/cxx-squid/src/test/java/org/sonar/cxx/lexer/CxxLexerWithPreprocessingTest.java b/cxx-squid/src/test/java/org/sonar/cxx/lexer/CxxLexerWithPreprocessingTest.java index 6b121cc5f4..c5e9445eaa 100644 --- a/cxx-squid/src/test/java/org/sonar/cxx/lexer/CxxLexerWithPreprocessingTest.java +++ b/cxx-squid/src/test/java/org/sonar/cxx/lexer/CxxLexerWithPreprocessingTest.java @@ -172,10 +172,10 @@ public void hashhash_arguments_with_whitespace_before_comma() { // such cases. // Corresponds to the Jira issue SONARPLUGINS-3060 List tokens = lexer.lex("#define FOOBAR 1\n" - + "#define CHECK(a, b) (( a ## b + 1 == 2))\n" - + "#if CHECK(FOO , BAR)\n" - + "yes\n" - + "#endif"); + + "#define CHECK(a, b) (( a ## b + 1 == 2))\n" + + "#if CHECK(FOO , BAR)\n" + + "yes\n" + + "#endif"); assertThat(tokens).hasSize(2); // yes + EOF assertThat(tokens, hasToken("yes", GenericTokenType.IDENTIFIER)); @@ -350,7 +350,7 @@ public void includes_are_working() { @Test public void macro_replacement_in_includes_is_working() { List tokens = lexer.lex("#define A \"B\"\n" - + "#include A\n"); + + "#include A\n"); assertThat(tokens).hasSize(1); // EOF } @@ -600,11 +600,11 @@ public void proper_expansion_of_function_like_macros_in_if_expressions() { @Test public void problem_with_chained_defined_expressions() { List tokens = lexer.lex("#define _C_\n" - + "#if !defined(_A_) && !defined(_B_) && !defined(_C_)\n" - + "truecase\n" - + "#else\n" - + "falsecase\n" - + "#endif"); + + "#if !defined(_A_) && !defined(_B_) && !defined(_C_)\n" + + "truecase\n" + + "#else\n" + + "falsecase\n" + + "#endif"); assertThat(tokens, hasToken("falsecase", GenericTokenType.IDENTIFIER)); assertThat(tokens).hasSize(2); // falsecase + EOF } @@ -612,13 +612,13 @@ public void problem_with_chained_defined_expressions() { @Test public void problem_evaluating_elif_expressions() { List tokens = lexer.lex("#define foo(a) 1\n" - + "#if 0\n" - + "body\n" - + "#elif foo(10)\n" - + "truecase\n" - + "#else\n" - + "falsecase\n" - + "endif\n"); + + "#if 0\n" + + "body\n" + + "#elif foo(10)\n" + + "truecase\n" + + "#else\n" + + "falsecase\n" + + "endif\n"); assertThat(tokens, hasToken("truecase", GenericTokenType.IDENTIFIER)); assertThat(tokens).hasSize(2); // truecase + EOF } @@ -631,19 +631,35 @@ public void built_in_macros() { } @Test - public void dont_look_at_subsequent_elif_clauses() { + public void dont_look_at_subsequent_clauses_if_elif() { // When a if clause has been evaluated to true, dont look at // subsequent elif clauses List tokens = lexer.lex("#define A 1\n" - + "#if A\n" - + "ifbody\n" - + "#elif A\n" - + "elifbody\n" - + "#endif"); + + "#if A\n" + + " ifbody\n" + + "#elif A\n" + + " elifbody\n" + + "#endif"); assertThat(tokens).hasSize(2); // ifbody + EOF assertThat(tokens, hasToken("ifbody", GenericTokenType.IDENTIFIER)); } + @Test + public void dont_look_at_subsequent_clauses_elif_elif() { + List tokens = lexer.lex("#define SDS_ARCH_darwin_15_i86\n" + + "#ifdef SDS_ARCH_freebsd_61_i86\n" + + " case1\n" + + "#elif defined(SDS_ARCH_darwin_15_i86)\n" + + " case2\n" + + "#elif defined(SDS_ARCH_winxp) || defined(SDS_ARCH_Interix)\n" + + " case3\n" + + "#else\n" + + " case4\n" + + "#endif\n"); + assertThat(tokens).hasSize(2); // case2 + EOF + assertThat(tokens, hasToken("case2", GenericTokenType.IDENTIFIER)); + } + //@Test public void hashhash_operator_problem() { // Corresponds to the Jira Issue SONARPLUGINS-3055. @@ -652,8 +668,8 @@ public void hashhash_operator_problem() { // After this, 0 and the rest of the number get never concatenated again. List tokens = lexer.lex("#define A B(cf)\n" - + "#define B(n) 0x##n\n" - + "A"); + + "#define B(n) 0x##n\n" + + "A"); assertThat(tokens, hasToken("0xcf", CxxKeyword.INT)); } }