From 17cd137c8a0fd8723d892be3a4ff2ffac551be46 Mon Sep 17 00:00:00 2001 From: "David P. Baker" Date: Thu, 1 Feb 2024 13:23:08 -0500 Subject: [PATCH] Refactoring in preparation of adding support for test names (#143) Extracted `Fact`. `ExpectedFact`, and `ReportedFact` to top level types, and adjusted their accessibility. Added `ExpectedFact.Reader` to contain logic to read expected facts from files, and `ConformanceTestReport.Builder` to build up a report. Added unit tests for `ExpectedFact.Reader`. Preparation for fixing #141. --- conformance-test-framework/build.gradle | 7 + .../conformance/ConformanceTestReport.java | 136 +++++++---- .../conformance/ConformanceTestRunner.java | 213 +----------------- .../jspecify/conformance/ExpectedFact.java | 142 ++++++++++++ .../java/org/jspecify/conformance/Fact.java | 42 ++++ .../jspecify/conformance/ReportedFact.java | 62 +++++ .../conformance/ExpectedFactTest.java | 61 +++++ src/test/java/tests/ConformanceTest.java | 19 +- 8 files changed, 415 insertions(+), 267 deletions(-) create mode 100644 conformance-test-framework/src/main/java/org/jspecify/conformance/ExpectedFact.java create mode 100644 conformance-test-framework/src/main/java/org/jspecify/conformance/Fact.java create mode 100644 conformance-test-framework/src/main/java/org/jspecify/conformance/ReportedFact.java create mode 100644 conformance-test-framework/src/test/java/org/jspecify/conformance/ExpectedFactTest.java diff --git a/conformance-test-framework/build.gradle b/conformance-test-framework/build.gradle index 5e278ee..633ff85 100644 --- a/conformance-test-framework/build.gradle +++ b/conformance-test-framework/build.gradle @@ -17,6 +17,13 @@ dependencies { api libs.guava implementation libs.jspecify implementation libs.truth + + testImplementation libs.junit + testImplementation libs.truth +} + +tasks.named('test', Test) { + useJUnit() } publishing { diff --git a/conformance-test-framework/src/main/java/org/jspecify/conformance/ConformanceTestReport.java b/conformance-test-framework/src/main/java/org/jspecify/conformance/ConformanceTestReport.java index 9c5eae4..6d62717 100644 --- a/conformance-test-framework/src/main/java/org/jspecify/conformance/ConformanceTestReport.java +++ b/conformance-test-framework/src/main/java/org/jspecify/conformance/ConformanceTestReport.java @@ -14,60 +14,32 @@ package org.jspecify.conformance; -import static com.google.common.collect.ImmutableListMultimap.flatteningToImmutableListMultimap; -import static com.google.common.collect.Iterables.concat; +import static com.google.common.collect.Maps.immutableEntry; import static com.google.common.collect.Multimaps.index; import static com.google.common.collect.Sets.union; +import static java.nio.charset.StandardCharsets.UTF_8; +import static java.nio.file.Files.readAllLines; import static java.util.Comparator.comparingLong; import static java.util.function.Predicate.not; +import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableListMultimap; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSortedSet; -import com.google.common.collect.Multimap; +import com.google.common.collect.ListMultimap; +import java.io.IOException; +import java.io.UncheckedIOException; import java.nio.file.Path; import java.util.Collection; import java.util.Formatter; -import org.jspecify.conformance.ConformanceTestRunner.ExpectedFact; -import org.jspecify.conformance.ConformanceTestRunner.Fact; -import org.jspecify.conformance.ConformanceTestRunner.ReportedFact; +import java.util.stream.Stream; -/** Represents the results of running {@link ConformanceTestRunner#runTests(Path, ImmutableList)} on a set of files. */ +/** + * Represents the result of running {@link ConformanceTestRunner#runTests(Path, ImmutableList)} on a + * set of files. + */ public final class ConformanceTestReport { - /** An empty report. */ - static final ConformanceTestReport EMPTY = - new ConformanceTestReport( - ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), ImmutableListMultimap.of()); - - /** Creates a report for a file. */ - static ConformanceTestReport forFile( - Path file, - ImmutableList reportedFacts, - ImmutableList expectedFacts) { - ImmutableListMultimap reportedFactsByLine = - index(reportedFacts, ReportedFact::getLineNumber); - ImmutableListMultimap matchingFacts = - expectedFacts.stream() - .collect( - flatteningToImmutableListMultimap( - f -> f, - expectedFact -> - reportedFactsByLine.get(expectedFact.getLineNumber()).stream() - .filter( - reportedFact -> reportedFact.matches(expectedFact.getFactText())))); - return new ConformanceTestReport( - ImmutableSortedSet.of(file), expectedFacts, reportedFacts, matchingFacts); - } - - /** Combines two reports into one. */ - static ConformanceTestReport combine(ConformanceTestReport left, ConformanceTestReport right) { - return new ConformanceTestReport( - union(left.files, right.files), - concat(left.expectedFactsByFile.values(), right.expectedFactsByFile.values()), - concat(left.reportedFactsByFile.values(), right.reportedFactsByFile.values()), - ImmutableListMultimap.copyOf( - concat(left.matchingFacts.entries(), right.matchingFacts.entries()))); - } private final ImmutableSortedSet files; private final ImmutableListMultimap expectedFactsByFile; @@ -75,14 +47,14 @@ static ConformanceTestReport combine(ConformanceTestReport left, ConformanceTest private final ImmutableListMultimap matchingFacts; private ConformanceTestReport( - Collection files, // Path unfortunately implements Iterable. - Iterable expectedFacts, - Iterable reportedFacts, - Multimap matchingFacts) { - this.files = ImmutableSortedSet.copyOf(files); - this.expectedFactsByFile = index(expectedFacts, Fact::getFile); - this.reportedFactsByFile = index(reportedFacts, Fact::getFile); - this.matchingFacts = ImmutableListMultimap.copyOf(matchingFacts); + ImmutableSortedSet files, + ImmutableListMultimap expectedFacts, + ImmutableListMultimap reportedFacts, + ImmutableListMultimap matchingFacts) { + this.files = files; + this.expectedFactsByFile = expectedFacts; + this.reportedFactsByFile = reportedFacts; + this.matchingFacts = matchingFacts; } /** @@ -160,4 +132,70 @@ private static void writeFact(Formatter report, Fact fact, String status) { report.format( "%s: %s:%d %s%n", status, fact.getFile(), fact.getLineNumber(), fact.getFactText()); } + + /** A builder for {@link ConformanceTestReport}s. */ + static class Builder { + private final ImmutableSortedSet.Builder files = ImmutableSortedSet.naturalOrder(); + private final ListMultimap reportedFacts = ArrayListMultimap.create(); + private final ImmutableList.Builder expectedFacts = ImmutableList.builder(); + private final ImmutableListMultimap.Builder matchingFacts = + ImmutableListMultimap.builder(); + private final ExpectedFact.Reader expectedFactReader = new ExpectedFact.Reader(); + private final Path testDirectory; + + /** + * Creates a builder. + * + * @param testDirectory the directory containing all {@linkplain #addFiles(Iterable, Iterable) + * files} with expected facts + */ + Builder(Path testDirectory) { + this.testDirectory = testDirectory; + } + + /** Adds test files and the facts reported for them. */ + void addFiles(Iterable files, Iterable reportedFacts) { + for (ReportedFact reportedFact : reportedFacts) { + this.reportedFacts.put(reportedFact.getFile(), reportedFact); + } + for (Path file : files) { + Path relativeFile = testDirectory.relativize(file); + this.files.add(relativeFile); + addExpectedFacts(relativeFile); + } + } + + private void addExpectedFacts(Path relativeFile) { + try { + ImmutableList expectedFactsInFile = + expectedFactReader.readExpectedFacts( + relativeFile, readAllLines(testDirectory.resolve(relativeFile), UTF_8)); + expectedFacts.addAll(expectedFactsInFile); + matchFacts(relativeFile, expectedFactsInFile).forEach(matchingFacts::put); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + private Stream> matchFacts( + Path file, ImmutableList expectedFactsInFile) { + ImmutableListMultimap reportedFactsByLine = + index(reportedFacts.get(file), ReportedFact::getLineNumber); + return expectedFactsInFile.stream() + .flatMap( + expectedFact -> + reportedFactsByLine.get(expectedFact.getLineNumber()).stream() + .filter(reportedFact -> reportedFact.matches(expectedFact)) + .map(reportedFact -> immutableEntry(expectedFact, reportedFact))); + } + + /** Builds the report. */ + ConformanceTestReport build() { + return new ConformanceTestReport( + files.build(), + index(expectedFacts.build(), Fact::getFile), + ImmutableListMultimap.copyOf(reportedFacts), + matchingFacts.build()); + } + } } diff --git a/conformance-test-framework/src/main/java/org/jspecify/conformance/ConformanceTestRunner.java b/conformance-test-framework/src/main/java/org/jspecify/conformance/ConformanceTestRunner.java index 54ad907..364b240 100644 --- a/conformance-test-framework/src/main/java/org/jspecify/conformance/ConformanceTestRunner.java +++ b/conformance-test-framework/src/main/java/org/jspecify/conformance/ConformanceTestRunner.java @@ -17,34 +17,21 @@ import static com.google.common.base.Strings.nullToEmpty; import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.Lists.partition; -import static com.google.common.collect.Multimaps.index; import static com.google.common.io.MoreFiles.asCharSink; import static com.google.common.io.MoreFiles.asCharSource; import static com.google.common.truth.Truth.assertThat; import static java.nio.charset.StandardCharsets.UTF_8; -import static java.nio.file.Files.readAllLines; import static java.nio.file.Files.walk; import static java.util.Arrays.stream; -import static java.util.Objects.requireNonNullElse; import static java.util.stream.Collectors.toList; -import static org.jspecify.conformance.ConformanceTestRunner.ExpectedFact.readExpectedFact; import com.google.common.base.Ascii; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableListMultimap; import java.io.IOException; import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; -import java.util.HashMap; -import java.util.List; -import java.util.ListIterator; -import java.util.Map; -import java.util.Objects; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import java.util.stream.Stream; -import org.jspecify.annotations.Nullable; /** An object that runs JSpecify conformance tests. */ public final class ConformanceTestRunner { @@ -79,8 +66,9 @@ public ConformanceTestRunner(Analyzer analyzer) { */ public ConformanceTestReport runTests(Path testDirectory, ImmutableList testDeps) throws IOException { + ConformanceTestReport.Builder report = new ConformanceTestReport.Builder(testDirectory); try (Stream paths = walk(testDirectory)) { - return paths + paths .filter(path -> path.toFile().isDirectory()) .flatMap( directory -> { @@ -89,54 +77,13 @@ public ConformanceTestReport runTests(Path testDirectory, ImmutableList te ? groups.flatMap(files -> partition(files, 1).stream()) : groups; }) - .flatMap(files -> analyzeFiles(files, testDeps, testDirectory)) - .reduce(ConformanceTestReport.EMPTY, ConformanceTestReport::combine); + .map(ImmutableList::copyOf) + .forEach( + files -> report.addFiles(files, analyzer.analyze(testDirectory, files, testDeps))); + return report.build(); } } - private Stream analyzeFiles( - List files, ImmutableList testDeps, Path testDirectory) { - ImmutableListMultimap reportedFactsByFile = - index( - analyzer.analyze(testDirectory, ImmutableList.copyOf(files), testDeps), - ReportedFact::getFile); - return files.stream() - .map(testDirectory::relativize) - .map( - file -> - ConformanceTestReport.forFile( - file, reportedFactsByFile.get(file), readExpectedFacts(file, testDirectory))); - } - - /** Reads {@link ExpectedFact}s from comments in a file. */ - private static ImmutableList readExpectedFacts(Path file, Path testDirectory) { - try { - ImmutableList.Builder expectedFacts = ImmutableList.builder(); - Map facts = new HashMap<>(); - for (ListIterator i = readAllLines(testDirectory.resolve(file), UTF_8).listIterator(); - i.hasNext(); ) { - String line = i.next(); - long lineNumber = i.nextIndex(); - Matcher matcher = EXPECTATION_COMMENT.matcher(line); - String fact = matcher.matches() ? readExpectedFact(matcher.group("expectation")) : null; - if (fact != null) { - facts.put(lineNumber, fact); - } else { - facts.forEach( - (factLineNumber, expectedFact) -> - expectedFacts.add( - new ExpectedFact(file, lineNumber, expectedFact, factLineNumber))); - facts.clear(); - } - } - return expectedFacts.build(); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - } - - private static final Pattern EXPECTATION_COMMENT = Pattern.compile("\\s*// (?.*)"); - private static Stream> javaFileGroups(Path directory) { ImmutableList files = javaFilesInDirectory(directory); return files.isEmpty() ? Stream.empty() : Stream.of(files); @@ -214,152 +161,4 @@ static Mode fromEnvironment() { } } } - - /** An expected or reported fact within a test input file. */ - public abstract static class Fact { - - private final Path file; - private final long lineNumber; - - protected Fact(Path file, long lineNumber) { - this.file = file; - this.lineNumber = lineNumber; - } - - /** The file path relative to the test source root. */ - public final Path getFile() { - return file; - } - - /** Returns the line number of the code in the source file to which this fact applies. */ - public final long getLineNumber() { - return lineNumber; - } - - /** The fact text. */ - public abstract String getFactText(); - } - - /** - * An assertion that the tool behaves in a way consistent with a specific fact about a line in the - * source code. Some of these facts indicate that according to the JSpecify specification, the - * code in question may have an error that should be reported to users; other expected facts are - * informational, such as the expected nullness-augmented type of an expression. - */ - public static final class ExpectedFact extends Fact { - private static final Pattern NULLNESS_MISMATCH = - Pattern.compile("jspecify_nullness_mismatch\\b.*"); - - private static final ImmutableList ASSERTION_PATTERNS = - ImmutableList.of( - NULLNESS_MISMATCH, - // TODO: wildcard types have whitespace - Pattern.compile("test:cannot-convert:\\S+ to \\S+"), - Pattern.compile("test:expression-type:[^:]+:.*"), - Pattern.compile("test:irrelevant-annotation:\\S+"), - Pattern.compile("test:sink-type:[^:]+:.*")); - - /** - * Returns an expected fact representing that the source type cannot be converted to the sink - * type in any world. - */ - public static String cannotConvert(String sourceType, String sinkType) { - return String.format("test:cannot-convert:%s to %s", sourceType, sinkType); - } - - /** Returns an expected fact representing an expected expression type. */ - public static String expressionType(String expressionType, String expression) { - return String.format("test:expression-type:%s:%s", expressionType, expression); - } - - /** Returns an expected fact representing that an annotation is not relevant. */ - public static String irrelevantAnnotation(String annotationType) { - return String.format("test:irrelevant-annotation:%s", annotationType); - } - - /** Returns an expected fact representing that an annotation is not relevant. */ - public static String sinkType(String sinkType, String sink) { - return String.format("test:sink-type:%s:%s", sinkType, sink); - } - - /** - * Returns {@code true} if {@code fact} is a legacy {@code jspecify_nullness_mismatch} - * assertion. - */ - public static boolean isNullnessMismatch(String fact) { - return NULLNESS_MISMATCH.matcher(fact).matches(); - } - - /** Read an expected fact from a line of either a source file or a report. */ - static @Nullable String readExpectedFact(String text) { - return ASSERTION_PATTERNS.stream().anyMatch(pattern -> pattern.matcher(text).matches()) - ? text - : null; - } - - private final String fact; - private final long factLineNumber; - - ExpectedFact(Path file, long lineNumber, String fact, long factLineNumber) { - super(file, lineNumber); - this.fact = fact; - this.factLineNumber = factLineNumber; - } - - @Override - public String getFactText() { - return fact; - } - - /** Returns the line number in the input file where the expected fact is. */ - public long getFactLineNumber() { - return factLineNumber; - } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (!(o instanceof ExpectedFact)) { - return false; - } - ExpectedFact that = (ExpectedFact) o; - return this.getFile().equals(that.getFile()) - && this.getLineNumber() == that.getLineNumber() - && this.fact.equals(that.fact); - } - - @Override - public int hashCode() { - return Objects.hash(getFile(), getLineNumber(), fact); - } - } - - /** A fact reported by the analysis under test. */ - public abstract static class ReportedFact extends Fact { - protected ReportedFact(Path file, long lineNumber) { - super(file, lineNumber); - } - - @Override - public String getFactText() { - return requireNonNullElse(expectedFact(), toString()); - } - - /** Returns true if this reported fact must match an {@link ExpectedFact}. */ - protected abstract boolean mustBeExpected(); - - /** Returns true if this reported fact matches the given expected fact. */ - protected boolean matches(String expectedFact) { - return expectedFact.equals(expectedFact()); - } - - /** Returns the equivalent expected fact. */ - protected abstract @Nullable String expectedFact(); - - /** Returns the message reported, without the file name or line number. */ - @Override - public abstract String toString(); - } } diff --git a/conformance-test-framework/src/main/java/org/jspecify/conformance/ExpectedFact.java b/conformance-test-framework/src/main/java/org/jspecify/conformance/ExpectedFact.java new file mode 100644 index 0000000..09e1c8a --- /dev/null +++ b/conformance-test-framework/src/main/java/org/jspecify/conformance/ExpectedFact.java @@ -0,0 +1,142 @@ +// Copyright 2023 The JSpecify Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package org.jspecify.conformance; + +import static com.google.common.base.MoreObjects.toStringHelper; +import static java.util.stream.Collectors.joining; + +import com.google.common.collect.ImmutableList; +import java.nio.file.Path; +import java.util.HashMap; +import java.util.List; +import java.util.ListIterator; +import java.util.Map; +import java.util.Objects; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * An assertion that the tool behaves in a way consistent with a specific fact about a line in the + * source code. Some of these facts indicate that according to the JSpecify specification, the code + * in question may have an error that should be reported to users; other expected facts are + * informational, such as the expected nullness-augmented type of an expression. + */ +public final class ExpectedFact extends Fact { + + private static final Pattern NULLNESS_MISMATCH = + Pattern.compile("jspecify_nullness_mismatch\\b.*"); + + private static final ImmutableList FACT_PATTERNS = + ImmutableList.of( + NULLNESS_MISMATCH, + // TODO: wildcard types have whitespace + Pattern.compile("test:cannot-convert:\\S+ to \\S+"), + Pattern.compile("test:expression-type:[^:]+:.*"), + Pattern.compile("test:irrelevant-annotation:\\S+"), + Pattern.compile("test:sink-type:[^:]+:.*")); + + private final String factText; + private final long factLineNumber; + + ExpectedFact(Path file, long lineNumber, String factText, long factLineNumber) { + super(file, lineNumber); + this.factText = factText; + this.factLineNumber = factLineNumber; + } + + /** + * Returns {@code true} if {@code fact} is a legacy {@code jspecify_nullness_mismatch} assertion. + */ + public boolean isNullnessMismatch() { + return NULLNESS_MISMATCH.matcher(getFactText()).matches(); + } + + @Override + protected String getFactText() { + return factText; + } + + /** Returns the line number in the input file where the expected fact is. */ + long getFactLineNumber() { + return factLineNumber; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof ExpectedFact)) { + return false; + } + ExpectedFact that = (ExpectedFact) o; + return this.getFile().equals(that.getFile()) + && this.getLineNumber() == that.getLineNumber() + && this.factText.equals(that.factText) + && this.factLineNumber == that.factLineNumber; + } + + @Override + public int hashCode() { + return Objects.hash(getFile(), getLineNumber(), factText); + } + + @Override + public String toString() { + return toStringHelper(this) + .add("file", getFile()) + .add("lineNumber", getLineNumber()) + .add("factText", factText) + .add("factLineNumber", factLineNumber) + .toString(); + } + + /** Reads {@link ExpectedFact}s from comments in a file. */ + static class Reader { + + private static final Pattern EXPECTATION_COMMENT = + Pattern.compile( + "\\s*// " + + ("(?" + + FACT_PATTERNS.stream().map(Pattern::pattern).collect(joining("|")) + + ")") + + "\\s*"); + + /** Reads expected facts from lines in a file. */ + ImmutableList readExpectedFacts(Path file, List lines) { + ImmutableList.Builder expectedFacts = ImmutableList.builder(); + Map facts = new HashMap<>(); + ListIterator i = lines.listIterator(); + while (i.hasNext()) { + String line = i.next(); + long lineNumber = i.nextIndex(); + Matcher matcher = EXPECTATION_COMMENT.matcher(line); + if (matcher.matches()) { + String fact = matcher.group("fact"); + if (fact != null) { + facts.put(lineNumber, fact.trim()); + } + } else { + facts.forEach( + (factLineNumber, factText) -> + expectedFacts.add(new ExpectedFact(file, lineNumber, factText, factLineNumber))); + facts.clear(); + } + } + // TODO(netdpb): Report an error if facts is not empty. + return expectedFacts.build(); + } + } +} diff --git a/conformance-test-framework/src/main/java/org/jspecify/conformance/Fact.java b/conformance-test-framework/src/main/java/org/jspecify/conformance/Fact.java new file mode 100644 index 0000000..2d2e1cd --- /dev/null +++ b/conformance-test-framework/src/main/java/org/jspecify/conformance/Fact.java @@ -0,0 +1,42 @@ +// Copyright 2023 The JSpecify Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package org.jspecify.conformance; + +import java.nio.file.Path; + +/** An expected or reported fact within a test input file. */ +abstract class Fact { + + private final Path file; + private final long lineNumber; + + protected Fact(Path file, long lineNumber) { + this.file = file; + this.lineNumber = lineNumber; + } + + /** The file path relative to the test source root. */ + final Path getFile() { + return file; + } + + /** Returns the line number of the code in the source file to which this fact applies. */ + final long getLineNumber() { + return lineNumber; + } + + /** The text form of the fact. */ + protected abstract String getFactText(); +} diff --git a/conformance-test-framework/src/main/java/org/jspecify/conformance/ReportedFact.java b/conformance-test-framework/src/main/java/org/jspecify/conformance/ReportedFact.java new file mode 100644 index 0000000..a66984f --- /dev/null +++ b/conformance-test-framework/src/main/java/org/jspecify/conformance/ReportedFact.java @@ -0,0 +1,62 @@ +// Copyright 2023 The JSpecify Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package org.jspecify.conformance; + +import java.nio.file.Path; + +/** A fact reported by the analysis under test. */ +public abstract class ReportedFact extends Fact { + + protected ReportedFact(Path file, long lineNumber) { + super(file, lineNumber); + } + + /** Returns true if this reported fact matches the given expected fact. */ + protected boolean matches(ExpectedFact expectedFact) { + return expectedFact.getFactText().equals(getFactText()); + } + + /** Returns true if this reported fact must match an {@link ExpectedFact}. */ + protected abstract boolean mustBeExpected(); + + /** + * Returns {@linkplain Fact#getFactText() fact text} representing that the source type cannot be + * converted to the sink type in any world. + */ + protected static String cannotConvert(String sourceType, String sinkType) { + return String.format("test:cannot-convert:%s to %s", sourceType, sinkType); + } + + /** Returns {@linkplain Fact#getFactText() fact text} representing an expected expression type. */ + protected static String expressionType(String expressionType, String expression) { + return String.format("test:expression-type:%s:%s", expressionType, expression); + } + + /** + * Returns {@linkplain Fact#getFactText() fact text} representing that an annotation is not + * relevant. + */ + protected static String irrelevantAnnotation(String annotationType) { + return String.format("test:irrelevant-annotation:%s", annotationType); + } + + /** + * Returns {@linkplain Fact#getFactText() fact text} representing that an annotation is not + * relevant. + */ + protected static String sinkType(String sinkType, String sink) { + return String.format("test:sink-type:%s:%s", sinkType, sink); + } +} diff --git a/conformance-test-framework/src/test/java/org/jspecify/conformance/ExpectedFactTest.java b/conformance-test-framework/src/test/java/org/jspecify/conformance/ExpectedFactTest.java new file mode 100644 index 0000000..0d69472 --- /dev/null +++ b/conformance-test-framework/src/test/java/org/jspecify/conformance/ExpectedFactTest.java @@ -0,0 +1,61 @@ +/* + * Copyright 2024 The JSpecify Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jspecify.conformance; + +import static com.google.common.truth.Truth.assertThat; +import static java.util.Arrays.asList; + +import com.google.common.collect.ImmutableList; +import java.nio.file.Path; +import java.nio.file.Paths; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class ExpectedFactTest { + + private static final Path FILE = Paths.get("path/to/File.java"); + + private final ExpectedFact.Reader reader = new ExpectedFact.Reader(); + + private ImmutableList readExpectedFacts(String... lines) { + return reader.readExpectedFacts(FILE, asList(lines)); + } + + @Test + public void readExpectedFacts() { + assertThat( + readExpectedFacts( + "// jspecify_nullness_mismatch ", + "// jspecify_nullness_mismatch plus_other_stuff ", + "line under test", + "// test:cannot-convert:type1 to type2 ", + "// test:expression-type:type2:expr2 ", + "// test:sink-type:type1:expr1 ", + "another line under test", + "// test:irrelevant-annotation:@Nullable ", + "yet another line under test")) + .containsExactly( + new ExpectedFact(FILE, 3, "jspecify_nullness_mismatch", 1), + new ExpectedFact(FILE, 3, "jspecify_nullness_mismatch plus_other_stuff", 2), + new ExpectedFact(FILE, 7, "test:cannot-convert:type1 to type2", 4), + new ExpectedFact(FILE, 7, "test:expression-type:type2:expr2", 5), + new ExpectedFact(FILE, 7, "test:sink-type:type1:expr1", 6), + new ExpectedFact(FILE, 9, "test:irrelevant-annotation:@Nullable", 8)) + .inOrder(); + } +} diff --git a/src/test/java/tests/ConformanceTest.java b/src/test/java/tests/ConformanceTest.java index b4a8253..e747560 100644 --- a/src/test/java/tests/ConformanceTest.java +++ b/src/test/java/tests/ConformanceTest.java @@ -19,11 +19,6 @@ import static java.util.Objects.requireNonNull; import static java.util.Objects.requireNonNullElse; import static java.util.stream.Collectors.joining; -import static org.jspecify.conformance.ConformanceTestRunner.ExpectedFact.cannotConvert; -import static org.jspecify.conformance.ConformanceTestRunner.ExpectedFact.expressionType; -import static org.jspecify.conformance.ConformanceTestRunner.ExpectedFact.irrelevantAnnotation; -import static org.jspecify.conformance.ConformanceTestRunner.ExpectedFact.isNullnessMismatch; -import static org.jspecify.conformance.ConformanceTestRunner.ExpectedFact.sinkType; import com.google.common.base.Joiner; import com.google.common.base.Splitter; @@ -45,7 +40,8 @@ import org.checkerframework.framework.test.diagnostics.DiagnosticKind; import org.jspecify.annotations.Nullable; import org.jspecify.conformance.ConformanceTestRunner; -import org.jspecify.conformance.ConformanceTestRunner.ReportedFact; +import org.jspecify.conformance.ExpectedFact; +import org.jspecify.conformance.ReportedFact; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -183,8 +179,8 @@ static final class DetailMessageReportedFact extends ReportedFact { } @Override - protected boolean matches(String expectedFact) { - if (isNullnessMismatch(expectedFact)) { + protected boolean matches(ExpectedFact expectedFact) { + if (expectedFact.isNullnessMismatch()) { return DEREFERENCE.equals(detailMessage.messageKey) || CANNOT_CONVERT_KEYS.contains(detailMessage.messageKey); } @@ -197,10 +193,11 @@ protected boolean mustBeExpected() { } @Override - protected @Nullable String expectedFact() { + protected String getFactText() { if (CANNOT_CONVERT_KEYS.contains(detailMessage.messageKey)) { if (detailMessage.messageArguments.size() < 2) { - return null; // The arguments must end with sourceType and sinkType. + // The arguments must end with sourceType and sinkType. + return toString(); } ImmutableList reversedArguments = detailMessage.messageArguments.reverse(); String sourceType = fixType(reversedArguments.get(1)); // penultimate @@ -225,7 +222,7 @@ protected boolean mustBeExpected() { return sinkType(sinkType, sink); } } - return null; + return toString(); } @Override