Skip to content

Commit

Permalink
Merge pull request #1645 from ivangalkin/preprocessor_context
Browse files Browse the repository at this point in the history 
CxxPreprocessor: fix handling of SquidAstVisitorContext
  • Loading branch information
@guwirth
guwirth authored Dec 31, 2018
2 parents ec2efd0 + c3304bd commit 4fe19d2
Show file tree
Hide file tree
Showing 7 changed files with 115 additions and 51 deletions.
72 changes: 40 additions & 32 deletions cxx-squid/src/main/java/org/sonar/cxx/preprocessor/CxxPreprocessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,7 @@
import com.sonar.sslr.impl.Parser;
import java.io.File;
import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.charset.Charset;
import java.nio.file.FileSystemNotFoundException;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
Expand All @@ -49,6 +45,7 @@
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -93,7 +90,6 @@ public class CxxPreprocessor extends Preprocessor {

private final CxxLanguage language;
private File currentContextFile;
private String rootFilePath;

private final Parser<Grammar> pplineParser;
private final MapChain<String, Macro> fixedMacros = new MapChain<>();
Expand All @@ -107,6 +103,7 @@ public class CxxPreprocessor extends Preprocessor {
private CxxCompilationUnitSettings compilationUnitSettings;
private final Multimap<String, Include> includedFiles = HashMultimap.create();
private final Multimap<String, Include> missingIncludeFiles = HashMultimap.create();
private boolean ctorInProgress = true;

private State currentFileState = new State(null);
private final Deque<State> globalStateStack = new LinkedList<>();
Expand Down Expand Up @@ -159,6 +156,7 @@ public CxxPreprocessor(SquidAstVisitorContext<Grammar> context,
}
} finally {
getMacros().setHighPrio(false);
ctorInProgress = false;
}
}

Expand Down Expand Up @@ -456,11 +454,19 @@ public PreprocessorAction process(List<Token> tokens) { //TODO: deprecated Prepr
Token token = tokens.get(0);
TokenType ttype = token.getType();

File file = getFileUnderAnalysis();
rootFilePath = file == null ? token.getURI().toString() : file.getAbsolutePath();
final String rootFilePath = getFileUnderAnalysis().getAbsolutePath();

if (context.getFile() != currentContextFile) {
// CxxPreprocessor::process() can be called a) while construction,
// b) for a new "physical" file or c) for #include directive.
// Make sure, that the following code is executed for a new "physical" file
// only.
final boolean processingNewSourceFile = !ctorInProgress && (context.getFile() != currentContextFile);

if (processingNewSourceFile) {
currentContextFile = context.getFile();
// In case "physical" file is preprocessed, SquidAstVisitorContext::getFile() cannot return null
// Did you forget to setup the mock properly?
Objects.requireNonNull(context.getFile(), "SquidAstVisitorContext::getFile() must be non-null");
compilationUnitSettings = conf.getCompilationUnitSettings(currentContextFile.getAbsolutePath());

if (compilationUnitSettings != null) {
Expand Down Expand Up @@ -640,9 +646,8 @@ public String expandFunctionLikeMacro(String macroName, List<Token> restTokens)
}

public Boolean expandHasIncludeExpression(AstNode exprAst) {
File file = getFileUnderAnalysis();
String filePath = file == null ? rootFilePath : file.getAbsolutePath();
return findIncludedFile(exprAst, exprAst.getToken(), filePath) != null;
final File file = getFileUnderAnalysis();
return findIncludedFile(exprAst, exprAst.getToken(), file.getAbsolutePath()) != null;
}

private boolean isCFile(String filePath) {
Expand Down Expand Up @@ -857,7 +862,6 @@ private Macro parseMacroDefinition(String macroDef) {

private File findIncludedFile(AstNode ast, Token token, String currFileName) {
String includedFileName = null;
File includedFile = null;
boolean quoted = false;

AstNode node = ast.getFirstDescendant(CppGrammar.includeBodyQuoted);
Expand Down Expand Up @@ -908,28 +912,34 @@ private File findIncludedFile(AstNode ast, Token token, String currFileName) {
}

if (includedFileName != null) {
File file = getFileUnderAnalysis();
String dir;
if (file != null) {
dir = file.getParent();
} else {
try {
dir = Paths.get(new URI(currFileName)).getParent().toString();
} catch (IllegalArgumentException | FileSystemNotFoundException | SecurityException | URISyntaxException e) {
dir = "";
}
}
includedFile = getCodeProvider().getSourceCodeFile(includedFileName, dir, quoted);
final File file = getFileUnderAnalysis();
final String dir = file.getParent();
return getCodeProvider().getSourceCodeFile(includedFileName, dir, quoted);
}

return includedFile;
return null;
}

private File getFileUnderAnalysis() {
if (currentFileState.includeUnderAnalysis == null) {
return context.getFile();
if (ctorInProgress) {
// a) CxxPreprocessor is parsing artificial #include and #define
// directives in order to initialize preprocessor with default macros
// and forced includes.
// This code is running in constructor of CxxPreprocessor. There is no
// information about the current file. Therefore return some artificial
// path under the project base directory.
return new File(conf.getBaseDir(), "CxxPreprocessorCtorInProgress.cpp").getAbsoluteFile();
} else if (currentFileState.includeUnderAnalysis != null) {
// b) CxxPreprocessor is called recursively in order to parse the #include
// directive. Return path to the included file.
return currentFileState.includeUnderAnalysis;
}
return currentFileState.includeUnderAnalysis;

// c) CxxPreprocessor is called in the ordinary mode: it is preprocessing the
// file, tracked in org.sonar.squidbridge.SquidAstVisitorContext. This file cannot
// be null. If it is null - you forgot to setup the test mock.
Objects.requireNonNull(context.getFile(), "SquidAstVisitorContext::getFile() must be non-null");
return context.getFile();
}

PreprocessorAction handleIfLine(AstNode ast, Token token, String filename) {
Expand Down Expand Up @@ -1068,7 +1078,7 @@ PreprocessorAction handleIncludeLine(AstNode ast, Token token, String filename,
File includedFile = findIncludedFile(ast, token, filename);

File currentFile = this.getFileUnderAnalysis();
if (currentFile != null && includedFile != null) {
if (includedFile != null) {
includedFiles.put(currentFile.getPath(), new Include(token.getLine(), includedFile.getAbsolutePath()));
}

Expand All @@ -1077,9 +1087,7 @@ PreprocessorAction handleIncludeLine(AstNode ast, Token token, String filename,
if (LOG.isDebugEnabled()) {
LOG.debug("[" + filename + ":" + token.getLine() + "]: cannot find include file '" + token.getValue() + "'");
}
if (currentFile != null) {
missingIncludeFiles.put(currentFile.getPath(), new Include(token.getLine(), token.getValue()));
}
missingIncludeFiles.put(currentFile.getPath(), new Include(token.getLine(), token.getValue()));
} else if (analysedFiles.add(includedFile.getAbsoluteFile())) {
if (LOG.isTraceEnabled()) {
LOG.trace("[{}:{}]: processing {}, resolved to file '{}'",
Expand Down
1 change: 1 addition & 0 deletions cxx-squid/src/main/java/org/sonar/cxx/preprocessor/MapChain.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ public V removeLowPrio(K key) {
*/
public void clearLowPrio() {
lowPrioMap.clear();
lowPrioDisabled.clear();
}

/**
Expand Down
11 changes: 10 additions & 1 deletion cxx-squid/src/test/java/org/sonar/cxx/lexer/CxxLexerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@
package org.sonar.cxx.lexer;

import com.sonar.sslr.api.GenericTokenType;
import com.sonar.sslr.api.Grammar;
import com.sonar.sslr.impl.Lexer;

import java.io.File;
import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.Arrays;
Expand All @@ -30,6 +33,8 @@
import org.junit.BeforeClass;
import org.junit.Test;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import org.sonar.cxx.CxxFileTesterHelper;
import org.sonar.cxx.CxxLanguage;
import org.sonar.cxx.api.CxxKeyword;
Expand All @@ -46,8 +51,12 @@ public class CxxLexerTest {

@BeforeClass
public static void init() {
File file = new File("snippet.cpp").getAbsoluteFile();
SquidAstVisitorContext<Grammar> context = mock(SquidAstVisitorContext.class);
when(context.getFile()).thenReturn(file);

CxxLanguage language = CxxFileTesterHelper.mockCxxLanguage();
CxxPreprocessor cxxpp = new CxxPreprocessor(mock(SquidAstVisitorContext.class), language);
CxxPreprocessor cxxpp = new CxxPreprocessor(context, language);
lexer = CxxLexer.create(cxxpp, new JoinStringsPreprocessor());
}

Expand Down
14 changes: 10 additions & 4 deletions cxx-squid/src/test/java/org/sonar/cxx/lexer/CxxLexerWithPreprocessingTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,16 @@ public class CxxLexerWithPreprocessingTest {

private static Lexer lexer;
private final CxxLanguage language;
private final SquidAstVisitorContext<Grammar> context;

public CxxLexerWithPreprocessingTest() {
language = CxxFileTesterHelper.mockCxxLanguage();
CxxPreprocessor cxxpp = new CxxPreprocessor(mock(SquidAstVisitorContext.class), language);

File file = new File("snippet.cpp").getAbsoluteFile();
context = mock(SquidAstVisitorContext.class);
when(context.getFile()).thenReturn(file);

CxxPreprocessor cxxpp = new CxxPreprocessor(context, language);
lexer = CxxLexer.create(cxxpp, new JoinStringsPreprocessor());
}

Expand Down Expand Up @@ -331,7 +337,7 @@ public void bodyless_defines() {
public void external_define() {
CxxConfiguration conf = new CxxConfiguration();
conf.setDefines(new String[]{"M body"});
CxxPreprocessor cxxpp = new CxxPreprocessor(mock(SquidAstVisitorContext.class), conf, language);
CxxPreprocessor cxxpp = new CxxPreprocessor(context, conf, language);
lexer = CxxLexer.create(conf, cxxpp, new JoinStringsPreprocessor());

List<Token> tokens = lexer.lex("M");
Expand All @@ -345,7 +351,7 @@ public void external_define() {
public void external_defines_with_params() {
CxxConfiguration conf = new CxxConfiguration();
conf.setDefines(new String[]{"minus(a, b) a - b"});
CxxPreprocessor cxxpp = new CxxPreprocessor(mock(SquidAstVisitorContext.class), conf, language);
CxxPreprocessor cxxpp = new CxxPreprocessor(context, conf, language);
lexer = CxxLexer.create(conf, cxxpp, new JoinStringsPreprocessor());

List<Token> tokens = lexer.lex("minus(1, 2)");
Expand Down Expand Up @@ -612,7 +618,7 @@ public void ignore_irrelevant_preprocessor_directives() {
public void externalMacrosCannotBeOverriden() {
CxxConfiguration conf = mock(CxxConfiguration.class);
when(conf.getDefines()).thenReturn(Arrays.asList("name goodvalue"));
CxxPreprocessor cxxpp = new CxxPreprocessor(mock(SquidAstVisitorContext.class), conf, language);
CxxPreprocessor cxxpp = new CxxPreprocessor(context, conf, language);
lexer = CxxLexer.create(conf, cxxpp);

List<Token> tokens = lexer.lex("#define name badvalue\n"
Expand Down
45 changes: 35 additions & 10 deletions cxx-squid/src/test/java/org/sonar/cxx/parser/CxxParserTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@

import com.sonar.sslr.api.AstNode;
import com.sonar.sslr.api.Grammar;
import com.sonar.sslr.impl.Parser;

import java.io.File;
import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import org.apache.commons.io.FileUtils;
Expand All @@ -38,7 +39,7 @@
import org.sonar.cxx.CxxFileTesterHelper;
import org.sonar.squidbridge.SquidAstVisitorContext;

public class CxxParserTest extends ParserBaseTestHelper {
public class CxxParserTest {

String errSources = "/parser/bad/error_recovery_declaration.cc";
String[] goodFiles = {"own", "VC", "cli", "cuda", "examples"};
Expand All @@ -54,7 +55,7 @@ public CxxParserTest() throws URISyntaxException {

@Test
public void testParsingOnDiverseSourceFiles() {
Collection<File> files = listFiles(goodFiles, new String[]{"cc", "cpp", "hpp"});
List<File> files = listFiles(goodFiles, new String[]{"cc", "cpp", "hpp"});
HashMap<String, Integer> map = new HashMap<String, Integer>() {
private static final long serialVersionUID = 6029310517902718597L;

Expand All @@ -76,7 +77,15 @@ public void testParsingOnDiverseSourceFiles() {
put("outbuf2.cpp", 2);
}
};

CxxConfiguration conf = new CxxConfiguration();
conf.setErrorRecoveryEnabled(false);

SquidAstVisitorContext<Grammar> context = mock(SquidAstVisitorContext.class);
Parser<Grammar> p = CxxParser.create(context, conf, CxxFileTesterHelper.mockCxxLanguage());

for (File file : files) {
when(context.getFile()).thenReturn(file);
AstNode root = p.parse(file);
CxxParser.finishedParsing(file);
if (map.containsKey(file.getName())) {
Expand All @@ -90,7 +99,7 @@ public void testParsingOnDiverseSourceFiles() {
@SuppressWarnings("unchecked")
@Test
public void testPreproccessorParsingOnDiverseSourceFiles() {
conf = new CxxConfiguration();
CxxConfiguration conf = new CxxConfiguration();
conf.setErrorRecoveryEnabled(false);
String baseDir = new File("src/test").getAbsolutePath();
conf.setBaseDir(baseDir);
Expand Down Expand Up @@ -118,9 +127,11 @@ public void testPreproccessorParsingOnDiverseSourceFiles() {
}
};

p = CxxParser.create(mock(SquidAstVisitorContext.class), conf, CxxFileTesterHelper.mockCxxLanguage());
Collection<File> files = listFiles(preprocessorFiles, new String[]{"cc", "cpp", "hpp", "h"});
SquidAstVisitorContext<Grammar> context = mock(SquidAstVisitorContext.class);
Parser<Grammar> p = CxxParser.create(context, conf, CxxFileTesterHelper.mockCxxLanguage());
List<File> files = listFiles(preprocessorFiles, new String[]{"cc", "cpp", "hpp", "h"});
for (File file : files) {
when(context.getFile()).thenReturn(file);
AstNode root = p.parse(file);
CxxParser.finishedParsing(file);
if (map.containsKey(file.getName())) {
Expand All @@ -139,13 +150,15 @@ public void testParsingInCCompatMode() { //ToDo: Fix this compatibility test - c
// This mode works if such a file causes parsing errors when the mode
// is switched off and doesn't, if the mode is switched on.

File cfile = (File) listFiles(cCompatibilityFiles, new String[]{"c"}).toArray()[0];
File cfile = listFiles(cCompatibilityFiles, new String[]{"c"}).get(0);

SquidAstVisitorContext<Grammar> context = mock(SquidAstVisitorContext.class);
when(context.getFile()).thenReturn(cfile);

CxxConfiguration conf = new CxxConfiguration();
conf.setErrorRecoveryEnabled(false);
conf.setCFilesPatterns(new String[]{""});
p = CxxParser.create(context, conf, CxxFileTesterHelper.mockCxxLanguage());
Parser<Grammar> p = CxxParser.create(context, conf, CxxFileTesterHelper.mockCxxLanguage());

AstNode root0 = p.parse(cfile);
assertThat(root0.getNumberOfChildren()).isEqualTo(2);
Expand All @@ -158,6 +171,14 @@ public void testParsingInCCompatMode() { //ToDo: Fix this compatibility test - c

@Test
public void testParseErrorRecoveryDisabled() {
SquidAstVisitorContext<Grammar> context = mock(SquidAstVisitorContext.class);
when(context.getFile()).thenReturn(erroneousSources);

CxxConfiguration conf = new CxxConfiguration();
conf.setErrorRecoveryEnabled(false);

Parser<Grammar> p = CxxParser.create(context, conf, CxxFileTesterHelper.mockCxxLanguage());

// The error recovery works, if:
// - a syntacticly incorrect file causes a parse error when recovery is disabled
assertThatThrownBy(() -> {
Expand All @@ -168,15 +189,19 @@ public void testParseErrorRecoveryDisabled() {
@SuppressWarnings("unchecked")
@Test
public void testParseErrorRecoveryEnabled() {
SquidAstVisitorContext<Grammar> context = mock(SquidAstVisitorContext.class);
when(context.getFile()).thenReturn(erroneousSources);

// The error recovery works, if:
// - but doesn't cause such an error if we run with default settings
CxxConfiguration conf = new CxxConfiguration();
conf.setErrorRecoveryEnabled(true);
p = CxxParser.create(mock(SquidAstVisitorContext.class), conf, CxxFileTesterHelper.mockCxxLanguage());
Parser<Grammar> p = CxxParser.create(context, conf, CxxFileTesterHelper.mockCxxLanguage());
AstNode root = p.parse(erroneousSources); //<-- this shouldn't throw now
assertThat(root.getNumberOfChildren()).isEqualTo(6);
}

private Collection<File> listFiles(String[] dirs, String[] extensions) {
private List<File> listFiles(String[] dirs, String[] extensions) {
List<File> files = new ArrayList<>();
for (String dir : dirs) {
files.addAll(FileUtils.listFiles(new File(rootDir, dir), extensions, true));
Expand Down
Loading

0 comments on commit 4fe19d2

Please sign in to comment.