From be303e85d8d8907f81196e51ac575175309f9403 Mon Sep 17 00:00:00 2001 From: Nima Karimipour Date: Thu, 14 Jul 2022 14:47:26 -0700 Subject: [PATCH 01/12] init --- .../java/edu/ucr/cs/riple/scanner/Config.java | 2 + .../cs/riple/scanner/DummyOptionsConfig.java | 2 + .../scanner/ErrorProneCLIFlagsConfig.java | 45 +++++++++---------- .../edu/ucr/cs/riple/scanner/Serializer.java | 17 +++---- .../edu/ucr/cs/riple/scanner/XMLUtil.java | 15 ++----- 5 files changed, 37 insertions(+), 44 deletions(-) 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..e4b8e704b 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 @@ -24,6 +24,7 @@ package edu.ucr.cs.riple.scanner; +import javax.annotation.Nonnull; import java.nio.file.Path; 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..59dccbfef 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 @@ -24,11 +24,13 @@ package edu.ucr.cs.riple.scanner; +import com.google.common.base.Preconditions; import com.google.errorprone.ErrorProneFlags; import java.io.IOException; 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,7 +39,7 @@ public class ErrorProneCLIFlagsConfig implements Config { - public final Path outputDirectory; + @Nonnull public final Path outputDirectory; public final boolean methodTrackerIsActive; public final boolean fieldTrackerIsActive; public final boolean callTrackerIsActive; @@ -46,34 +48,26 @@ public class ErrorProneCLIFlagsConfig implements Config { 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); - } + Preconditions.checkNotNull( + configFilePath, + "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; + Preconditions.checkNotNull( + outputDirectoryPathInString, + "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 +80,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 +103,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..e9b335f22 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,9 @@ 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 +117,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); From 3f129ec1df84fd9af70931ed68f6fe9f0c5d1d9c Mon Sep 17 00:00:00 2001 From: Nima Karimipour Date: Thu, 14 Jul 2022 14:59:27 -0700 Subject: [PATCH 02/12] format fix --- .../main/java/edu/ucr/cs/riple/scanner/DummyOptionsConfig.java | 2 +- scanner/src/main/java/edu/ucr/cs/riple/scanner/Serializer.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) 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 e4b8e704b..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 @@ -24,8 +24,8 @@ package edu.ucr.cs.riple.scanner; -import javax.annotation.Nonnull; import java.nio.file.Path; +import javax.annotation.Nonnull; public class DummyOptionsConfig implements Config { 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 e9b335f22..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,7 +53,6 @@ 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(Config config) { Path outputDirectory = config.getOutputDirectory(); this.fieldGraphPath = outputDirectory.resolve(FIELD_GRAPH_FILE_NAME); From fe8992b6541de8c7128d8f621b399980c7a794e5 Mon Sep 17 00:00:00 2001 From: Nima Karimipour Date: Mon, 18 Jul 2022 10:31:32 -0700 Subject: [PATCH 03/12] added correct config test --- .../cs/riple/scanner/ConfigurationTest.java | 119 ++++++++++++++++++ .../tools/SerializationTestHelper.java | 22 +++- .../cs/riple/scanner/SampleClassForTest.java | 12 ++ 3 files changed, 151 insertions(+), 2 deletions(-) create mode 100644 scanner/src/test/java/edu/ucr/cs/riple/scanner/ConfigurationTest.java create mode 100644 scanner/src/test/resources/edu/ucr/cs/riple/scanner/SampleClassForTest.java 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..0f88e6736 --- /dev/null +++ b/scanner/src/test/java/edu/ucr/cs/riple/scanner/ConfigurationTest.java @@ -0,0 +1,119 @@ +/* + * 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() { + tester = + new SerializationTestHelper<>(root) + .setArgs( + Arrays.asList( + "-d", temporaryFolder.getRoot().getAbsolutePath(), "-Xep:Scanner:ERROR")) + .setOutputFileNameAndHeader("Unknown", "Unknown") + .addSourceFile("SampleClassForTest.java") + .setFactory(factory); + tester.doTest( + "Error in Scanner Checker configuration, should be set with via error prone flag: (-XepOpt:Scanner:ConfigPath)"); + } + + @Test + public void checkOutputDirIsNonnull() { + Path config = root.resolve("scanner.xml"); + try { + Files.createDirectories(root); + Files.createFile(config); + DocumentBuilderFactory docFactory = DocumentBuilderFactory.newInstance(); + try { + 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 in 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.doTest("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..fe0fb80dc 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 @@ -58,6 +58,15 @@ public SerializationTestHelper addSourceLines(String path, String... lines) { return this; } + @SuppressWarnings("ResultOfMethodCallIgnored") + public SerializationTestHelper addSourceFile(String path) { + // This class is inside "tools" directory and tests are written in the parent directory of this + // class. + path = "../" + path; + compilationTestHelper.addSourceFile(path); + return this; + } + @SafeVarargs public final SerializationTestHelper setExpectedOutputs(T... outputs) { this.expectedOutputs = ImmutableList.copyOf(outputs); @@ -86,7 +95,7 @@ public SerializationTestHelper setOutputFileNameAndHeader(String fileName, St return this; } - public void doTest() { + public void doTest(String expectedErrorMessage) { Preconditions.checkNotNull(factory, "Factory cannot be null"); Preconditions.checkNotNull(fileName, "File name cannot be null"); Path outputPath = outputDir.resolve(fileName); @@ -95,11 +104,20 @@ public void doTest() { } catch (IOException ignored) { throw new RuntimeException("Failed to delete older file at: " + outputPath); } - compilationTestHelper.doTest(); + try { + compilationTestHelper.doTest(); + } catch (Throwable e) { + assert e.getMessage().contains(expectedErrorMessage); + return; + } List actualOutputs = readActualOutputs(outputPath); compare(actualOutputs); } + public void doTest() { + doTest(null); + } + private void compare(List actualOutput) { List notFound = new ArrayList<>(); for (T o : expectedOutputs) { 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..42d07ad66 --- /dev/null +++ b/scanner/src/test/resources/edu/ucr/cs/riple/scanner/SampleClassForTest.java @@ -0,0 +1,12 @@ +package edu.ucr.cs.riple.scanner.testdata; + +public class SampleClassForTest { + + Object field; + + public void foo() {} + + public Object bar() { + return new Object(); + } +} From 35108b4744a45e8925cbbf26541500ca78a93318 Mon Sep 17 00:00:00 2001 From: Nima Karimipour Date: Mon, 18 Jul 2022 10:37:47 -0700 Subject: [PATCH 04/12] fix field visibility --- .../cs/riple/scanner/ErrorProneCLIFlagsConfig.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) 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 59dccbfef..4b32f2e1c 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 @@ -39,12 +39,12 @@ public class ErrorProneCLIFlagsConfig implements Config { - @Nonnull 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"; From 5b0ef67beb58f19d4d6679a2a85044eb1eb7f801 Mon Sep 17 00:00:00 2001 From: Nima Karimipour Date: Mon, 18 Jul 2022 11:10:16 -0700 Subject: [PATCH 05/12] fix null expected message --- .../edu/ucr/cs/riple/scanner/tools/SerializationTestHelper.java | 2 ++ 1 file changed, 2 insertions(+) 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 fe0fb80dc..4cf87f5d2 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 @@ -107,6 +107,8 @@ public void doTest(String expectedErrorMessage) { try { compilationTestHelper.doTest(); } catch (Throwable e) { + Preconditions.checkNotNull( + expectedErrorMessage, "Encountered an unexpected error: " + e.getMessage()); assert e.getMessage().contains(expectedErrorMessage); return; } From 16a79dc5c8531df881ca538f87cd5e84d0a10303 Mon Sep 17 00:00:00 2001 From: Nima Karimipour Date: Mon, 18 Jul 2022 11:12:22 -0700 Subject: [PATCH 06/12] add comments --- .../test/java/edu/ucr/cs/riple/scanner/ConfigurationTest.java | 3 +++ 1 file changed, 3 insertions(+) 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 index 0f88e6736..a710b2bb4 100644 --- a/scanner/src/test/java/edu/ucr/cs/riple/scanner/ConfigurationTest.java +++ b/scanner/src/test/java/edu/ucr/cs/riple/scanner/ConfigurationTest.java @@ -68,6 +68,7 @@ public void setup() { @Test public void checkExceptionIsThrownIfConfigPathNotSet() { + // -XepOpt:Scanner:ConfigPath is not set should expect an error. tester = new SerializationTestHelper<>(root) .setArgs( @@ -82,12 +83,14 @@ public void checkExceptionIsThrownIfConfigPathNotSet() { @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"); From 67e9fee091a925c3ffe20ffefcb766e4bfcfa8f3 Mon Sep 17 00:00:00 2001 From: Nima Karimipour Date: Mon, 18 Jul 2022 13:21:45 -0700 Subject: [PATCH 07/12] comment update --- .../ucr/cs/riple/scanner/tools/SerializationTestHelper.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 4cf87f5d2..4055829f1 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 @@ -60,8 +60,7 @@ public SerializationTestHelper addSourceLines(String path, String... lines) { @SuppressWarnings("ResultOfMethodCallIgnored") public SerializationTestHelper addSourceFile(String path) { - // This class is inside "tools" directory and tests are written in the parent directory of this - // class. + // 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; From 8cc8f182b61b5a41487cdb66b811762702640222 Mon Sep 17 00:00:00 2001 From: Nima Karimipour Date: Mon, 18 Jul 2022 13:22:05 -0700 Subject: [PATCH 08/12] reformat --- .../ucr/cs/riple/scanner/tools/SerializationTestHelper.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 4055829f1..2b529aa89 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 @@ -60,7 +60,9 @@ public SerializationTestHelper addSourceLines(String path, String... lines) { @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. + // 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; From 42b84b5fc2f8224b9eb1f8fa29009aa3b579febd Mon Sep 17 00:00:00 2001 From: Nima Karimipour Date: Mon, 18 Jul 2022 13:29:44 -0700 Subject: [PATCH 09/12] error message update --- .../test/java/edu/ucr/cs/riple/scanner/ConfigurationTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index a710b2bb4..e991198c8 100644 --- a/scanner/src/test/java/edu/ucr/cs/riple/scanner/ConfigurationTest.java +++ b/scanner/src/test/java/edu/ucr/cs/riple/scanner/ConfigurationTest.java @@ -101,7 +101,7 @@ public void checkOutputDirIsNonnull() { StreamResult result = new StreamResult(config.toFile()); transformer.transform(source, result); } catch (ParserConfigurationException | TransformerException e) { - throw new RuntimeException("Error happened in writing config.", e); + throw new RuntimeException("Error happened while writing config.", e); } } catch (IOException ex) { throw new UncheckedIOException(ex); From 8e6d9195de2b5859ecb0c48b96af912dde0d8b1e Mon Sep 17 00:00:00 2001 From: Nima Karimipour Date: Mon, 18 Jul 2022 16:57:32 -0700 Subject: [PATCH 10/12] refined exception types --- .../scanner/ErrorProneCLIFlagsConfig.java | 15 +++---- .../cs/riple/scanner/ConfigurationTest.java | 5 ++- .../tools/SerializationTestHelper.java | 39 ++++++++++--------- 3 files changed, 33 insertions(+), 26 deletions(-) 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 4b32f2e1c..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 @@ -24,7 +24,6 @@ package edu.ucr.cs.riple.scanner; -import com.google.common.base.Preconditions; import com.google.errorprone.ErrorProneFlags; import java.io.IOException; import java.nio.file.Files; @@ -50,9 +49,10 @@ public class ErrorProneCLIFlagsConfig implements Config { public ErrorProneCLIFlagsConfig(ErrorProneFlags flags) { String configFilePath = flags.get(FL_CONFIG_PATH).orElse(null); - Preconditions.checkNotNull( - configFilePath, - "Error in Scanner Checker configuration, should be set with via error prone flag: (-XepOpt:Scanner:ConfigPath)"); + 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(); @@ -64,9 +64,10 @@ public ErrorProneCLIFlagsConfig(ErrorProneFlags flags) { } String outputDirectoryPathInString = XMLUtil.getValueFromTag(document, "/scanner/path", String.class).orElse(null); - Preconditions.checkNotNull( - outputDirectoryPathInString, - "Output path cannot be null, should be set it in config file within tag"); + 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) 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 index e991198c8..26d3b2dfb 100644 --- a/scanner/src/test/java/edu/ucr/cs/riple/scanner/ConfigurationTest.java +++ b/scanner/src/test/java/edu/ucr/cs/riple/scanner/ConfigurationTest.java @@ -78,6 +78,7 @@ public void checkExceptionIsThrownIfConfigPathNotSet() { .addSourceFile("SampleClassForTest.java") .setFactory(factory); tester.doTest( + IllegalStateException.class, "Error in Scanner Checker configuration, should be set with via error prone flag: (-XepOpt:Scanner:ConfigPath)"); } @@ -117,6 +118,8 @@ public void checkOutputDirIsNonnull() { .setOutputFileNameAndHeader("Unknown", "Unknown") .addSourceFile("SampleClassForTest.java") .setFactory(factory); - tester.doTest("Output path cannot be null, should be set it in config file within tag"); + tester.doTest( + 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 2b529aa89..fed80b363 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,6 +49,8 @@ public class SerializationTestHelper { private String fileName; private String header; + private Path outputFilePath; + public SerializationTestHelper(Path outputDir) { this.outputDir = outputDir; } @@ -96,29 +99,29 @@ public SerializationTestHelper setOutputFileNameAndHeader(String fileName, St return this; } - public void doTest(String expectedErrorMessage) { + public 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); - } - try { - compilationTestHelper.doTest(); - } catch (Throwable e) { - Preconditions.checkNotNull( - expectedErrorMessage, "Encountered an unexpected error: " + e.getMessage()); - assert e.getMessage().contains(expectedErrorMessage); - return; + throw new RuntimeException("Failed to delete older file at: " + outputFilePath); } - List actualOutputs = readActualOutputs(outputPath); - compare(actualOutputs); + } + + public void doTest(Class exception, String expectedErrorMessage) { + String fullExpectedMessage = "Caused by: " + exception.getName() + ": " + expectedErrorMessage; + prepareTest(); + AssertionError ex = assertThrows(AssertionError.class, () -> compilationTestHelper.doTest()); + assert ex.getMessage().contains(fullExpectedMessage); } public void doTest() { - doTest(null); + prepareTest(); + compilationTestHelper.doTest(); + List actualOutputs = readActualOutputs(); + compare(actualOutputs); } private void compare(List actualOutput) { @@ -153,16 +156,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: " From 2977561248bebbfeddb2e8a8a66822f48761aa5c Mon Sep 17 00:00:00 2001 From: Nima Karimipour Date: Mon, 18 Jul 2022 17:10:00 -0700 Subject: [PATCH 11/12] enhance class test --- .../ucr/cs/riple/scanner/ClassInfoTest.java | 25 ++++++++++++++++++- .../cs/riple/scanner/SampleClassForTest.java | 16 +++++++++++- 2 files changed, 39 insertions(+), 2 deletions(-) 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/resources/edu/ucr/cs/riple/scanner/SampleClassForTest.java b/scanner/src/test/resources/edu/ucr/cs/riple/scanner/SampleClassForTest.java index 42d07ad66..55fbdb994 100644 --- a/scanner/src/test/resources/edu/ucr/cs/riple/scanner/SampleClassForTest.java +++ b/scanner/src/test/resources/edu/ucr/cs/riple/scanner/SampleClassForTest.java @@ -4,9 +4,23 @@ public class SampleClassForTest { Object field; - public void foo() {} + 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(); +} From bb44daccd3265033e0021b61c297d84e426b041e Mon Sep 17 00:00:00 2001 From: Nima Karimipour Date: Tue, 19 Jul 2022 08:56:08 -0700 Subject: [PATCH 12/12] Added javadoc --- .../cs/riple/scanner/ConfigurationTest.java | 4 +- .../tools/SerializationTestHelper.java | 58 ++++++++++++++++++- 2 files changed, 58 insertions(+), 4 deletions(-) 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 index 26d3b2dfb..d67d33557 100644 --- a/scanner/src/test/java/edu/ucr/cs/riple/scanner/ConfigurationTest.java +++ b/scanner/src/test/java/edu/ucr/cs/riple/scanner/ConfigurationTest.java @@ -77,7 +77,7 @@ public void checkExceptionIsThrownIfConfigPathNotSet() { .setOutputFileNameAndHeader("Unknown", "Unknown") .addSourceFile("SampleClassForTest.java") .setFactory(factory); - tester.doTest( + tester.doTestWithExpectingError( IllegalStateException.class, "Error in Scanner Checker configuration, should be set with via error prone flag: (-XepOpt:Scanner:ConfigPath)"); } @@ -118,7 +118,7 @@ public void checkOutputDirIsNonnull() { .setOutputFileNameAndHeader("Unknown", "Unknown") .addSourceFile("SampleClassForTest.java") .setFactory(factory); - tester.doTest( + 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 fed80b363..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 @@ -55,12 +55,25 @@ 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 @@ -71,35 +84,68 @@ public SerializationTestHelper addSourceFile(String 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 prepareTest() { + private void prepareTest() { Preconditions.checkNotNull(factory, "Factory cannot be null"); Preconditions.checkNotNull(fileName, "File name cannot be null"); outputFilePath = outputDir.resolve(fileName); @@ -110,13 +156,21 @@ public void prepareTest() { } } - public void doTest(Class exception, String expectedErrorMessage) { + /** + * 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();