diff --git a/scanner/src/main/java/edu/ucr/cs/riple/scanner/Config.java b/scanner/src/main/java/edu/ucr/cs/riple/scanner/Config.java index 6c67b9b13..353abbd0c 100644 --- a/scanner/src/main/java/edu/ucr/cs/riple/scanner/Config.java +++ b/scanner/src/main/java/edu/ucr/cs/riple/scanner/Config.java @@ -26,6 +26,7 @@ import com.google.common.base.Preconditions; import java.nio.file.Path; +import javax.annotation.Nonnull; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; @@ -49,6 +50,7 @@ public interface Config { Serializer getSerializer(); + @Nonnull Path getOutputDirectory(); class Builder { diff --git a/scanner/src/main/java/edu/ucr/cs/riple/scanner/DummyOptionsConfig.java b/scanner/src/main/java/edu/ucr/cs/riple/scanner/DummyOptionsConfig.java index 1fd9cdf0c..43a312189 100644 --- a/scanner/src/main/java/edu/ucr/cs/riple/scanner/DummyOptionsConfig.java +++ b/scanner/src/main/java/edu/ucr/cs/riple/scanner/DummyOptionsConfig.java @@ -25,6 +25,7 @@ package edu.ucr.cs.riple.scanner; import java.nio.file.Path; +import javax.annotation.Nonnull; public class DummyOptionsConfig implements Config { @@ -56,6 +57,7 @@ public Serializer getSerializer() { throw new IllegalStateException(ERROR_MESSAGE); } + @Nonnull @Override public Path getOutputDirectory() { throw new IllegalStateException(ERROR_MESSAGE); diff --git a/scanner/src/main/java/edu/ucr/cs/riple/scanner/ErrorProneCLIFlagsConfig.java b/scanner/src/main/java/edu/ucr/cs/riple/scanner/ErrorProneCLIFlagsConfig.java index 416b38276..6288ca718 100644 --- a/scanner/src/main/java/edu/ucr/cs/riple/scanner/ErrorProneCLIFlagsConfig.java +++ b/scanner/src/main/java/edu/ucr/cs/riple/scanner/ErrorProneCLIFlagsConfig.java @@ -29,6 +29,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import javax.annotation.Nonnull; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; @@ -37,43 +38,37 @@ public class ErrorProneCLIFlagsConfig implements Config { - public final Path outputDirectory; - public final boolean methodTrackerIsActive; - public final boolean fieldTrackerIsActive; - public final boolean callTrackerIsActive; - public final boolean classTrackerIsActive; - public final Serializer serializer; + @Nonnull private final Path outputDirectory; + private final boolean methodTrackerIsActive; + private final boolean fieldTrackerIsActive; + private final boolean callTrackerIsActive; + private final boolean classTrackerIsActive; + private final Serializer serializer; static final String EP_FL_NAMESPACE = "Scanner"; static final String FL_CONFIG_PATH = EP_FL_NAMESPACE + ":ConfigPath"; - public ErrorProneCLIFlagsConfig() { - this.methodTrackerIsActive = false; - this.fieldTrackerIsActive = false; - this.callTrackerIsActive = false; - this.classTrackerIsActive = false; - this.outputDirectory = null; - this.serializer = new Serializer(this); - } - public ErrorProneCLIFlagsConfig(ErrorProneFlags flags) { String configFilePath = flags.get(FL_CONFIG_PATH).orElse(null); - Document document = null; - if (configFilePath != null) { - try { - DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); - DocumentBuilder builder = factory.newDocumentBuilder(); - document = builder.parse(Files.newInputStream(Paths.get(configFilePath))); - document.normalize(); - } catch (IOException | SAXException | ParserConfigurationException e) { - throw new RuntimeException("Error in reading/parsing config at path: " + configFilePath, e); - } + if (configFilePath == null) { + throw new IllegalStateException( + "Error in Scanner Checker configuration, should be set with via error prone flag: (-XepOpt:Scanner:ConfigPath)"); + } + Document document; + try { + DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + DocumentBuilder builder = factory.newDocumentBuilder(); + document = builder.parse(Files.newInputStream(Paths.get(configFilePath))); + document.normalize(); + } catch (IOException | SAXException | ParserConfigurationException e) { + throw new RuntimeException("Error in reading/parsing config at path: " + configFilePath, e); } String outputDirectoryPathInString = XMLUtil.getValueFromTag(document, "/scanner/path", String.class).orElse(null); - // Here we do not throw an exception if outputDirectoryPathInString is null , since this - // constructor can still be called when the checker is not activated. - this.outputDirectory = - (outputDirectoryPathInString != null) ? Paths.get(outputDirectoryPathInString) : null; + if (outputDirectoryPathInString == null) { + throw new IllegalArgumentException( + "Output path cannot be null, should be set it in config file within tag"); + } + this.outputDirectory = Paths.get(outputDirectoryPathInString); this.methodTrackerIsActive = XMLUtil.getValueFromAttribute(document, "/scanner/method", "active", Boolean.class) .orElse(false); @@ -86,7 +81,7 @@ public ErrorProneCLIFlagsConfig(ErrorProneFlags flags) { this.classTrackerIsActive = XMLUtil.getValueFromAttribute(document, "/scanner/class", "active", Boolean.class) .orElse(false); - this.serializer = (this.outputDirectory == null) ? null : new Serializer(this); + this.serializer = new Serializer(this); } public boolean callTrackerIsActive() { @@ -109,6 +104,7 @@ public Serializer getSerializer() { return serializer; } + @Nonnull public Path getOutputDirectory() { return outputDirectory; } diff --git a/scanner/src/main/java/edu/ucr/cs/riple/scanner/Serializer.java b/scanner/src/main/java/edu/ucr/cs/riple/scanner/Serializer.java index 500cddcf0..80358f918 100644 --- a/scanner/src/main/java/edu/ucr/cs/riple/scanner/Serializer.java +++ b/scanner/src/main/java/edu/ucr/cs/riple/scanner/Serializer.java @@ -53,8 +53,8 @@ public class Serializer { public static final String METHOD_INFO_FILE_NAME = "method_info.tsv"; public static final String CLASS_INFO_FILE_NAME = "class_info.tsv"; - public Serializer(ErrorProneCLIFlagsConfig config) { - Path outputDirectory = config.outputDirectory; + public Serializer(Config config) { + Path outputDirectory = config.getOutputDirectory(); this.fieldGraphPath = outputDirectory.resolve(FIELD_GRAPH_FILE_NAME); this.callGraphPath = outputDirectory.resolve(CALL_GRAPH_FILE_NAME); this.methodInfoPath = outputDirectory.resolve(METHOD_INFO_FILE_NAME); @@ -116,19 +116,19 @@ private void initializeFile(Path path, String header) { } /** Initializes every file which will be re-generated in the new run of NullAway. */ - private void initializeOutputFiles(ErrorProneCLIFlagsConfig config) { + private void initializeOutputFiles(Config config) { try { - Files.createDirectories(config.outputDirectory); - if (config.callTrackerIsActive) { + Files.createDirectories(config.getOutputDirectory()); + if (config.callTrackerIsActive()) { initializeFile(callGraphPath, TrackerNode.header()); } - if (config.fieldTrackerIsActive) { + if (config.fieldTrackerIsActive()) { initializeFile(fieldGraphPath, TrackerNode.header()); } - if (config.methodTrackerIsActive) { + if (config.methodTrackerIsActive()) { initializeFile(methodInfoPath, MethodInfo.header()); } - if (config.classTrackerIsActive) { + if (config.classTrackerIsActive()) { initializeFile(classInfoPath, ClassInfo.header()); } } catch (IOException e) { diff --git a/scanner/src/main/java/edu/ucr/cs/riple/scanner/XMLUtil.java b/scanner/src/main/java/edu/ucr/cs/riple/scanner/XMLUtil.java index 55d2bbcb6..f981e78bd 100644 --- a/scanner/src/main/java/edu/ucr/cs/riple/scanner/XMLUtil.java +++ b/scanner/src/main/java/edu/ucr/cs/riple/scanner/XMLUtil.java @@ -9,7 +9,6 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import javax.annotation.Nullable; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; @@ -29,17 +28,14 @@ public class XMLUtil { * Helper method for reading attributes of node located at /key_1/key_2/.../key_n (in the form of * {@code Xpath} query) from a {@link Document}. * - * @param doc XML object to read values from, if null passed, the default value will be returned. + * @param doc XML object to read values from. * @param key Key to locate the value, can be nested in the form of {@code Xpath} query (e.g. * /key1/key2:.../key_n). * @param klass Class type of the value in doc. * @return The value in the specified keychain cast to the class type given in parameter. */ public static DefaultXMLValueProvider getValueFromAttribute( - @Nullable Document doc, String key, String attr, Class klass) { - if (doc == null) { - return new DefaultXMLValueProvider<>(null, klass); - } + Document doc, String key, String attr, Class klass) { try { XPath xPath = XPathFactory.newInstance().newXPath(); Node node = (Node) xPath.compile(key).evaluate(doc, XPathConstants.NODE); @@ -57,17 +53,14 @@ public static DefaultXMLValueProvider getValueFromAttribute( * Helper method for reading value of a node located at /key_1/key_2/.../key_n (in the form of * {@code Xpath} query) from a {@link Document}. * - * @param doc XML object to read values from, if null passed, the default value will be returned. + * @param doc XML object to read values from. * @param key Key to locate the value, can be nested in the form of {@code Xpath} query (e.g. * /key1/key2/.../key_n). * @param klass Class type of the value in doc. * @return The value in the specified keychain cast to the class type given in parameter. */ public static DefaultXMLValueProvider getValueFromTag( - @Nullable Document doc, String key, Class klass) { - if (doc == null) { - return new DefaultXMLValueProvider<>(null, klass); - } + Document doc, String key, Class klass) { try { XPath xPath = XPathFactory.newInstance().newXPath(); Node node = (Node) xPath.compile(key).evaluate(doc, XPathConstants.NODE); diff --git a/scanner/src/test/java/edu/ucr/cs/riple/scanner/ClassInfoTest.java b/scanner/src/test/java/edu/ucr/cs/riple/scanner/ClassInfoTest.java index 2a10601cb..a60e57048 100644 --- a/scanner/src/test/java/edu/ucr/cs/riple/scanner/ClassInfoTest.java +++ b/scanner/src/test/java/edu/ucr/cs/riple/scanner/ClassInfoTest.java @@ -51,10 +51,33 @@ public ClassInfoTest() { } @Test - public void BasicTest() { + public void basicTest() { tester .addSourceLines("edu/ucr/A.java", "package edu.ucr;", "public class A", "{", "}") .setExpectedOutputs(new ClassInfoDisplay("edu.ucr.A", "edu/ucr/A.java")) .doTest(); } + + @Test + public void checkClassesAreWrittenInFlatName() { + tester + .addSourceFile("SampleClassForTest.java") + .setExpectedOutputs( + new ClassInfoDisplay( + "edu.ucr.cs.riple.scanner.testdata.SampleClassForTest", + "edu/ucr/cs/riple/scanner/SampleClassForTest.java"), + new ClassInfoDisplay( + "edu.ucr.cs.riple.scanner.testdata.SampleClassForTest$Inner", + "edu/ucr/cs/riple/scanner/SampleClassForTest.java"), + new ClassInfoDisplay( + "edu.ucr.cs.riple.scanner.testdata.SampleClassForTest$1InnerMethod", + "edu/ucr/cs/riple/scanner/SampleClassForTest.java"), + new ClassInfoDisplay( + "edu.ucr.cs.riple.scanner.testdata.SampleClassForTest$1InnerMethod$1", + "edu/ucr/cs/riple/scanner/SampleClassForTest.java"), + new ClassInfoDisplay( + "edu.ucr.cs.riple.scanner.testdata.Run", + "edu/ucr/cs/riple/scanner/SampleClassForTest.java")) + .doTest(); + } } diff --git a/scanner/src/test/java/edu/ucr/cs/riple/scanner/ConfigurationTest.java b/scanner/src/test/java/edu/ucr/cs/riple/scanner/ConfigurationTest.java new file mode 100644 index 000000000..d67d33557 --- /dev/null +++ b/scanner/src/test/java/edu/ucr/cs/riple/scanner/ConfigurationTest.java @@ -0,0 +1,125 @@ +/* + * MIT License + * + * Copyright (c) 2022 Nima Karimipour + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package edu.ucr.cs.riple.scanner; + +import edu.ucr.cs.riple.scanner.tools.ClassInfoDisplay; +import edu.ucr.cs.riple.scanner.tools.Display; +import edu.ucr.cs.riple.scanner.tools.DisplayFactory; +import edu.ucr.cs.riple.scanner.tools.SerializationTestHelper; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Arrays; +import javax.xml.parsers.DocumentBuilder; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; +import javax.xml.transform.Transformer; +import javax.xml.transform.TransformerException; +import javax.xml.transform.TransformerFactory; +import javax.xml.transform.dom.DOMSource; +import javax.xml.transform.stream.StreamResult; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.w3c.dom.Document; +import org.w3c.dom.Element; + +/** Includes tests that check wrong configurations are correctly reported. */ +@RunWith(JUnit4.class) +public class ConfigurationTest { + + @Rule public final TemporaryFolder temporaryFolder = new TemporaryFolder(); + // Just a dummy factory to run the tests with, tests will not even start with a null factory. + protected DisplayFactory factory = values -> new ClassInfoDisplay("Unknown", "Unknown"); + protected SerializationTestHelper tester; + protected Path root; + + @Before + public void setup() { + root = Paths.get(temporaryFolder.getRoot().getAbsolutePath()); + } + + @Test + public void checkExceptionIsThrownIfConfigPathNotSet() { + // -XepOpt:Scanner:ConfigPath is not set should expect an error. + tester = + new SerializationTestHelper<>(root) + .setArgs( + Arrays.asList( + "-d", temporaryFolder.getRoot().getAbsolutePath(), "-Xep:Scanner:ERROR")) + .setOutputFileNameAndHeader("Unknown", "Unknown") + .addSourceFile("SampleClassForTest.java") + .setFactory(factory); + tester.doTestWithExpectingError( + IllegalStateException.class, + "Error in Scanner Checker configuration, should be set with via error prone flag: (-XepOpt:Scanner:ConfigPath)"); + } + + @Test + public void checkOutputDirIsNonnull() { + // ... tag is not set and should expect an error. + Path config = root.resolve("scanner.xml"); + try { + Files.createDirectories(root); + Files.createFile(config); + DocumentBuilderFactory docFactory = DocumentBuilderFactory.newInstance(); + try { + // Make empty tag with no as child. + DocumentBuilder docBuilder = docFactory.newDocumentBuilder(); + Document doc = docBuilder.newDocument(); + Element rootElement = doc.createElement("scanner"); + doc.appendChild(rootElement); + TransformerFactory transformerFactory = TransformerFactory.newInstance(); + Transformer transformer = transformerFactory.newTransformer(); + DOMSource source = new DOMSource(doc); + StreamResult result = new StreamResult(config.toFile()); + transformer.transform(source, result); + } catch (ParserConfigurationException | TransformerException e) { + throw new RuntimeException("Error happened while writing config.", e); + } + } catch (IOException ex) { + throw new UncheckedIOException(ex); + } + tester = + new SerializationTestHelper<>(root) + .setArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-Xep:Scanner:ERROR", + "-XepOpt:Scanner:ConfigPath=" + config)) + .setOutputFileNameAndHeader("Unknown", "Unknown") + .addSourceFile("SampleClassForTest.java") + .setFactory(factory); + tester.doTestWithExpectingError( + IllegalArgumentException.class, + "Output path cannot be null, should be set it in config file within tag"); + } +} diff --git a/scanner/src/test/java/edu/ucr/cs/riple/scanner/tools/SerializationTestHelper.java b/scanner/src/test/java/edu/ucr/cs/riple/scanner/tools/SerializationTestHelper.java index d2e68cf1b..4924f1c79 100644 --- a/scanner/src/test/java/edu/ucr/cs/riple/scanner/tools/SerializationTestHelper.java +++ b/scanner/src/test/java/edu/ucr/cs/riple/scanner/tools/SerializationTestHelper.java @@ -24,6 +24,7 @@ package edu.ucr.cs.riple.scanner.tools; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.fail; import com.google.common.base.Preconditions; @@ -48,55 +49,132 @@ public class SerializationTestHelper { private String fileName; private String header; + private Path outputFilePath; + public SerializationTestHelper(Path outputDir) { this.outputDir = outputDir; } + /** + * Adds source code to the list of inputs. This method is part of the builder pattern. + * + * @param path Relative path to src directory where the given source code should exist. + * @param lines Lines of source code. + * @return Receiver of the call. + */ @SuppressWarnings("ResultOfMethodCallIgnored") public SerializationTestHelper addSourceLines(String path, String... lines) { compilationTestHelper.addSourceLines(path, lines); return this; } + /** + * Adds source code to the list of inputs. This method is part of the builder pattern. + * + * @param path Path to resource. + * @return Receiver of the call. + */ + @SuppressWarnings("ResultOfMethodCallIgnored") + public SerializationTestHelper addSourceFile(String path) { + // This class is inside "tools" package, which means compilationTestHelper will try to use a + // corresponding "tools" directory within test/resources/[...]/scanner. We prepend ".." to the + // path, to escape this non-existent directory. + path = "../" + path; + compilationTestHelper.addSourceFile(path); + return this; + } + + /** + * Sets the expected output. Any unseen / unexpected output will result to a failure in the test. + * This method is part of the builder pattern. + * + * @param outputs Expected output. + * @return Receiver of the call. + */ @SafeVarargs public final SerializationTestHelper setExpectedOutputs(T... outputs) { this.expectedOutputs = ImmutableList.copyOf(outputs); return this; } + /** + * If called, no output should be expected while running the test. This method is part of the + * builder pattern. + * + * @return Receiver of the call. + */ public SerializationTestHelper expectNoOutput() { this.expectedOutputs = ImmutableList.of(); return this; } + /** + * Creates the actual {@link Scanner} with the given arguments. This method is part of the builder + * pattern and should be called before any other method. + * + * @return Receiver of the call. + */ public SerializationTestHelper setArgs(List args) { compilationTestHelper = CompilationTestHelper.newInstance(Scanner.class, getClass()).setArgs(args); return this; } + /** + * Sets factory. This method is part of the builder pattern. + * + * @param factory Factory instance. + * @return Receiver of the call. + */ public SerializationTestHelper setFactory(DisplayFactory factory) { this.factory = factory; return this; } + /** + * Sets file name and the expected header of output file. This method is part of the builder + * pattern. + * + * @param fileName Output file name. + * @param header Expected header. + * @return Receiver of the call. + */ public SerializationTestHelper setOutputFileNameAndHeader(String fileName, String header) { this.fileName = fileName; this.header = header; return this; } - public void doTest() { + private void prepareTest() { Preconditions.checkNotNull(factory, "Factory cannot be null"); Preconditions.checkNotNull(fileName, "File name cannot be null"); - Path outputPath = outputDir.resolve(fileName); + outputFilePath = outputDir.resolve(fileName); try { - Files.deleteIfExists(outputPath); + Files.deleteIfExists(outputFilePath); } catch (IOException ignored) { - throw new RuntimeException("Failed to delete older file at: " + outputPath); + throw new RuntimeException("Failed to delete older file at: " + outputFilePath); } + } + + /** + * Runs the testing with expecting to encounter a specific error. + * + * @param exception Expected Exception to be raised by running the test. + * @param expectedErrorMessage Expected message to be printed by running the test. + */ + public void doTestWithExpectingError( + Class exception, String expectedErrorMessage) { + String fullExpectedMessage = "Caused by: " + exception.getName() + ": " + expectedErrorMessage; + prepareTest(); + AssertionError ex = assertThrows(AssertionError.class, () -> compilationTestHelper.doTest()); + assert ex.getMessage().contains(fullExpectedMessage); + } + + /** Runs the test. */ + public void doTest() { + prepareTest(); compilationTestHelper.doTest(); - List actualOutputs = readActualOutputs(outputPath); + List actualOutputs = readActualOutputs(); compare(actualOutputs); } @@ -132,16 +210,16 @@ private void compare(List actualOutput) { fail(errorMessage.toString()); } - private List readActualOutputs(Path outputPath) { + private List readActualOutputs() { List outputs = new ArrayList<>(); BufferedReader reader; try { - reader = Files.newBufferedReader(outputPath, Charset.defaultCharset()); + reader = Files.newBufferedReader(outputFilePath, Charset.defaultCharset()); String actualHeader = reader.readLine(); if (!header.equals(actualHeader)) { fail( "Expected header of " - + outputPath.getFileName() + + outputFilePath.getFileName() + " to be: " + header + "\nBut found: " diff --git a/scanner/src/test/resources/edu/ucr/cs/riple/scanner/SampleClassForTest.java b/scanner/src/test/resources/edu/ucr/cs/riple/scanner/SampleClassForTest.java new file mode 100644 index 000000000..55fbdb994 --- /dev/null +++ b/scanner/src/test/resources/edu/ucr/cs/riple/scanner/SampleClassForTest.java @@ -0,0 +1,26 @@ +package edu.ucr.cs.riple.scanner.testdata; + +public class SampleClassForTest { + + Object field; + + static class Inner {} + + public void foo() { + class InnerMethod { + Run r = + new Run() { + @Override + public void exec() {} + }; + } + } + + public Object bar() { + return new Object(); + } +} + +interface Run { + void exec(); +}