From ac75a3fb61a91fd27f56e2cd3db461062b3d8825 Mon Sep 17 00:00:00 2001 From: Ivan Galkin Date: Tue, 22 May 2018 23:33:13 +0200 Subject: [PATCH] JoinStringsPreprocessor: store original tokens for highlighting Fix highlighting issue #1468 * `JoinStringsPreprocessor` replaces adjacent string Tokens with a single concatenated one * Previously the original Tokens were lost and proper highligting for multiline adjacent string literals were impossible * Solution: a) store original Tokens as Trivia b) if such trivia was found during highlighting: highlight preserved Tokens * ALSO minor refactoring and simplification of `JoinStringsPreprocessor` --- .../visitors/CxxHighlighterVisitor.java | 20 ++++- .../sensors/visitors/CxxHighlighterTest.java | 32 ++++++- .../org/sonar/cxx/sensors/highlighter.cc | 7 ++ .../preprocessor/JoinStringsPreprocessor.java | 88 +++++++++---------- 4 files changed, 98 insertions(+), 49 deletions(-) diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/visitors/CxxHighlighterVisitor.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/visitors/CxxHighlighterVisitor.java index 961efe85e6..51062461fa 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/visitors/CxxHighlighterVisitor.java +++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/visitors/CxxHighlighterVisitor.java @@ -24,6 +24,9 @@ import com.sonar.sslr.api.Grammar; import com.sonar.sslr.api.Token; import com.sonar.sslr.api.Trivia; + +import java.util.List; +import java.util.Optional; import java.util.regex.Matcher; import java.util.regex.Pattern; import javax.annotation.Nullable; @@ -140,6 +143,12 @@ public void leaveFile(@Nullable AstNode astNode) { } } + + private Optional getTriviaWithConcatenatedLiterals(Token stringToken) { + return stringToken.getTrivia().stream() + .filter(t -> t.isSkippedText() && CxxTokenType.STRING.equals(t.getToken().getType())).findFirst(); + } + @Override public void visitToken(Token token) { if (!token.isGeneratedCode()) { @@ -148,8 +157,17 @@ public void visitToken(Token token) { last = highlight(last, new TokenLocation(token), TypeOfText.CONSTANT); } else if (token.getType() instanceof CxxKeyword) { last = highlight(last, new TokenLocation(token), TypeOfText.KEYWORD); - } else if (token.getType().equals(CxxTokenType.STRING) || token.getType().equals(CxxTokenType.CHARACTER)) { + } else if (token.getType().equals(CxxTokenType.CHARACTER)) { last = highlight(last, new TokenLocation(token), TypeOfText.STRING); + } else if (token.getType().equals(CxxTokenType.STRING)) { + Optional triviaWithConcatenatedLiterals = getTriviaWithConcatenatedLiterals(token); + if (!triviaWithConcatenatedLiterals.isPresent()) { + last = highlight(last, new TokenLocation(token), TypeOfText.STRING); + } else { + for (Token concatenatedLiterals : triviaWithConcatenatedLiterals.get().getTokens()) { + last = highlight(last, new TokenLocation(concatenatedLiterals), TypeOfText.STRING); + } + } } for (Trivia trivia : token.getTrivia()) { diff --git a/cxx-sensors/src/test/java/org/sonar/cxx/sensors/visitors/CxxHighlighterTest.java b/cxx-sensors/src/test/java/org/sonar/cxx/sensors/visitors/CxxHighlighterTest.java index d721c97307..9652108327 100644 --- a/cxx-sensors/src/test/java/org/sonar/cxx/sensors/visitors/CxxHighlighterTest.java +++ b/cxx-sensors/src/test/java/org/sonar/cxx/sensors/visitors/CxxHighlighterTest.java @@ -83,8 +83,36 @@ public void stringLiteral() { checkOnRange(86, 24, 6, TypeOfText.STRING); // u"..." checkOnRange(87, 24, 6, TypeOfText.STRING); // U"..." - checkOnRange(89, 24, 13, TypeOfText.STRING); // "hello" " world" - checkOnRange(90, 24, 13, TypeOfText.STRING); // u"" "hello world" + // "hello" " world" + checkOnRange(89, 24, 7, TypeOfText.STRING); + checkOnRange(89, 32, 8, TypeOfText.STRING); + + // u"" "hello world" + checkOnRange(90, 24, 3, TypeOfText.STRING); + checkOnRange(90, 28, 13, TypeOfText.STRING); + + // /*comment1*/ u"" /*comment2*/ "hello world" /*comment3*/; // issue #996 + checkOnRange(91, 24, 12, TypeOfText.COMMENT); + checkOnRange(91, 37, 3, TypeOfText.STRING); + checkOnRange(91, 41, 12, TypeOfText.COMMENT); + checkOnRange(91, 54, 13, TypeOfText.STRING); + checkOnRange(91, 68, 12, TypeOfText.COMMENT); + checkOnRange(91, 82, 13, TypeOfText.COMMENT); + + // /*comment4*/ "hello" + // /*comment5*/ " world" /*comment6*/; + checkOnRange(93, 24, 12, TypeOfText.COMMENT); + checkOnRange(93, 37, 7, TypeOfText.STRING); + checkOnRange(94, 24, 12, TypeOfText.COMMENT); + checkOnRange(94, 37, 8, TypeOfText.STRING); + checkOnRange(94, 46, 12, TypeOfText.COMMENT); + + // "hello" + // "Mary" + // "Lou"; + checkOnRange(96, 25, 7, TypeOfText.STRING); + checkOnRange(97, 25, 6, TypeOfText.STRING); + checkOnRange(98, 25, 5, TypeOfText.STRING); } @Test diff --git a/cxx-sensors/src/test/resources/org/sonar/cxx/sensors/highlighter.cc b/cxx-sensors/src/test/resources/org/sonar/cxx/sensors/highlighter.cc index 3380513898..2ba0cbb99a 100644 --- a/cxx-sensors/src/test/resources/org/sonar/cxx/sensors/highlighter.cc +++ b/cxx-sensors/src/test/resources/org/sonar/cxx/sensors/highlighter.cc @@ -89,6 +89,13 @@ void test3() const char *t6 = "hello" " world"; const wchar_t *t7 = u"" "hello world"; const wchar_t *t8 = /*comment1*/ u"" /*comment2*/ "hello world" /*comment3*/; // issue #996 + + const char *t9 = /*comment4*/ "hello" + /*comment5*/ " world" /*comment6*/; + + const char *t10 = "hello" + "Mary" + "Lou"; } /* EOF */ diff --git a/cxx-squid/src/main/java/org/sonar/cxx/preprocessor/JoinStringsPreprocessor.java b/cxx-squid/src/main/java/org/sonar/cxx/preprocessor/JoinStringsPreprocessor.java index 55573d40e2..724ab131d4 100644 --- a/cxx-squid/src/main/java/org/sonar/cxx/preprocessor/JoinStringsPreprocessor.java +++ b/cxx-squid/src/main/java/org/sonar/cxx/preprocessor/JoinStringsPreprocessor.java @@ -23,66 +23,62 @@ import com.sonar.sslr.api.Preprocessor; import com.sonar.sslr.api.PreprocessorAction; import com.sonar.sslr.api.Token; +import com.sonar.sslr.api.Trivia; + import java.util.ArrayList; import java.util.Collections; +import java.util.Iterator; import java.util.List; import org.sonar.cxx.api.CxxTokenType; -public class JoinStringsPreprocessor extends Preprocessor { //@todo deprecated Preprocessor +// @todo deprecated PreprocessorAction +public class JoinStringsPreprocessor extends Preprocessor { @Override - public PreprocessorAction process(List tokens) { //@todo deprecated PreprocessorAction - Token token = tokens.get(0); - - if (token.getType().equals(CxxTokenType.STRING)) { - - // Joining string literals (C++ Standard, "2.2 Phases of translation, Phase 6") - StringBuilder newStr = null; - int numberOfStrings = 1; - boolean isGenerated = token.isGeneratedCode(); + // @todo deprecated PreprocessorAction + public PreprocessorAction process(List tokens) { - for (;;) { - Token nextToken = tokens.get(numberOfStrings); - if (!nextToken.getType().equals(CxxTokenType.STRING)) { - if (newStr != null) { - newStr.append('\"'); - } - break; - } - if (newStr == null) { - newStr = new StringBuilder(); - newStr.append('\"'); - newStr.append(stripQuotes(token.getValue())); - } - newStr.append(stripQuotes(nextToken.getValue())); - if (nextToken.isGeneratedCode()) { - isGenerated = true; - } - numberOfStrings++; - } - - if (newStr != null) { - List tokensToInject = new ArrayList<>(); - tokensToInject.add( - Token.builder() - .setLine(token.getLine()) - .setColumn(token.getColumn()) - .setURI(token.getURI()) - .setType(CxxTokenType.STRING) - .setValueAndOriginalValue(newStr.toString()) - .setGeneratedCode(isGenerated) - .build() - ); - //@todo deprecated PreprocessorAction - return new PreprocessorAction(numberOfStrings, Collections.emptyList(), tokensToInject); + int nrOfAdjacentStringLiterals = 0; + boolean isGenerated = false; + for (Token t : tokens) { + if (!CxxTokenType.STRING.equals(t.getType())) { + break; } + nrOfAdjacentStringLiterals++; + isGenerated |= t.isGeneratedCode(); + } - return PreprocessorAction.NO_OPERATION; //@todo deprecated PreprocessorAction + if (nrOfAdjacentStringLiterals < 2) { + // @todo deprecated PreprocessorAction + return PreprocessorAction.NO_OPERATION; } - return PreprocessorAction.NO_OPERATION; //@todo deprecated PreprocessorAction + + // Concatenate adjacent string literals + // (C++ Standard, "2.2 Phases of translation, Phase 6") + List concatenatedTokens = new ArrayList<>(tokens.subList(0, nrOfAdjacentStringLiterals)); + String concatentedLiteral = concatenateStringLiterals(concatenatedTokens); + Trivia trivia = Trivia.createSkippedText(concatenatedTokens); + Token firstToken = tokens.get(0); + Token tokenToInject = Token.builder().setLine(firstToken.getLine()).setColumn(firstToken.getColumn()) + .setURI(firstToken.getURI()).setType(CxxTokenType.STRING).setValueAndOriginalValue(concatentedLiteral) + .setGeneratedCode(isGenerated).build(); + + // @todo deprecated PreprocessorAction + return new PreprocessorAction(nrOfAdjacentStringLiterals, Collections.singletonList(trivia), + Collections.singletonList(tokenToInject)); } private static String stripQuotes(String str) { return str.substring(str.indexOf('"') + 1, str.lastIndexOf('"')); } + + private static String concatenateStringLiterals(List concatenatedTokens) { + StringBuilder sb = new StringBuilder(); + sb.append("\""); + for (Token t : concatenatedTokens) { + sb.append(stripQuotes(t.getValue())); + } + sb.append("\""); + return sb.toString(); + } }