Skip to content

Commit e04612c

Browse files
graememorganError Prone Team
authored and
Error Prone Team
committed
Sniff out the canonical constructor using detective work rather than a flag which isn't there.
PiperOrigin-RevId: 737645805
1 parent c139e7f commit e04612c

File tree

3 files changed

+111
-0
lines changed

3 files changed

+111
-0
lines changed

check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java

+23
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,16 @@
2222
import static com.google.common.collect.ImmutableList.toImmutableList;
2323
import static com.google.common.collect.ImmutableSet.toImmutableSet;
2424
import static com.google.common.collect.Iterables.getOnlyElement;
25+
import static com.google.common.collect.MoreCollectors.onlyElement;
2526
import static com.google.common.collect.Streams.stream;
27+
import static com.google.common.collect.Streams.zip;
2628
import static com.google.errorprone.VisitorState.memoize;
2729
import static com.google.errorprone.matchers.JUnitMatchers.JUNIT4_RUN_WITH_ANNOTATION;
2830
import static com.google.errorprone.matchers.Matchers.isSubtypeOf;
2931
import static com.sun.tools.javac.code.Scope.LookupKind.NON_RECURSIVE;
3032
import static java.util.Objects.requireNonNull;
3133
import static java.util.stream.Collectors.toCollection;
34+
import static javax.lang.model.element.ElementKind.CONSTRUCTOR;
3235

3336
import com.github.benmanes.caffeine.cache.Cache;
3437
import com.github.benmanes.caffeine.cache.Caffeine;
@@ -833,11 +836,31 @@ private static boolean isFinal(Symbol symbol) {
833836
/**
834837
* Returns whether the given {@link Symbol} is a record, a record's canonical constructor or a
835838
* member that is part of a record's state vector.
839+
*
840+
* <p>Health warning: some things are flagged within a compilation, but won't be flagged across
841+
* compilation boundaries, like canonical constructors.
836842
*/
837843
public static boolean isRecord(Symbol symbol) {
838844
return (symbol.flags() & RECORD_FLAG) == RECORD_FLAG;
839845
}
840846

847+
/** Finds the canonical constructor on a record. */
848+
public static MethodSymbol canonicalConstructor(ClassSymbol record, VisitorState state) {
849+
var fieldTypes =
850+
record.getRecordComponents().stream().map(rc -> rc.type).collect(toImmutableList());
851+
return stream(record.members().getSymbols(s -> s.getKind() == CONSTRUCTOR))
852+
.map(c -> (MethodSymbol) c)
853+
.filter(
854+
c ->
855+
c.getParameters().size() == fieldTypes.size()
856+
&& zip(
857+
c.getParameters().stream(),
858+
fieldTypes.stream(),
859+
(a, b) -> isSameType(a.type, b, state))
860+
.allMatch(x -> x))
861+
.collect(onlyElement());
862+
}
863+
841864
/**
842865
* Determines whether a symbol has an annotation of the given type. This includes annotations
843866
* inherited from superclasses due to {@code @Inherited}.

check_api/src/test/java/com/google/errorprone/util/ASTHelpersTest.java

+61
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@
2121
import static com.google.common.truth.Truth.assertWithMessage;
2222
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
2323
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
24+
import static com.google.errorprone.util.ASTHelpers.canonicalConstructor;
2425
import static com.google.errorprone.util.ASTHelpers.getStartPosition;
26+
import static com.google.errorprone.util.ASTHelpers.getSymbol;
2527
import static com.google.errorprone.util.ASTHelpers.hasAnnotation;
2628
import static java.lang.annotation.ElementType.FIELD;
2729
import static java.lang.annotation.ElementType.LOCAL_VARIABLE;
@@ -2104,4 +2106,63 @@ void test(Integer[] i, List<String> s) {
21042106
""")
21052107
.doTest();
21062108
}
2109+
2110+
@BugPattern(summary = "", severity = WARNING)
2111+
public static final class CanonicalConstructorFinder extends BugChecker
2112+
implements MethodTreeMatcher {
2113+
@Override
2114+
public Description matchMethod(MethodTree tree, VisitorState state) {
2115+
return canonicalConstructor((ClassSymbol) getSymbol(tree).owner, state) == getSymbol(tree)
2116+
? describeMatch(tree)
2117+
: Description.NO_MATCH;
2118+
}
2119+
}
2120+
2121+
@Test
2122+
public void canonicalConstructors_found() {
2123+
CompilationTestHelper.newInstance(CanonicalConstructorFinder.class, getClass())
2124+
.addSourceLines(
2125+
"Test.java",
2126+
"""
2127+
import java.util.List;
2128+
import java.util.Set;
2129+
2130+
class Test {
2131+
record A(int x) {}
2132+
2133+
record B(long y) {
2134+
// BUG: Diagnostic contains:
2135+
B {}
2136+
}
2137+
2138+
record C(long y) {
2139+
// BUG: Diagnostic contains:
2140+
C {}
2141+
2142+
C(int z) {
2143+
this((long) (z + 1));
2144+
}
2145+
}
2146+
2147+
record D(List<Integer> xs) {
2148+
// BUG: Diagnostic contains:
2149+
D {}
2150+
2151+
D(Set<Integer> s) {
2152+
this(List.of(1));
2153+
}
2154+
}
2155+
2156+
record E() {
2157+
// BUG: Diagnostic contains:
2158+
E {}
2159+
2160+
E(int x) {
2161+
this();
2162+
}
2163+
}
2164+
}
2165+
""")
2166+
.doTest();
2167+
}
21072168
}

core/src/test/java/com/google/errorprone/bugpatterns/argumentselectiondefects/ArgumentSelectionDefectCheckerTest.java

+27
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
*/
1616
package com.google.errorprone.bugpatterns.argumentselectiondefects;
1717

18+
import static com.google.common.truth.TruthJUnit.assume;
19+
1820
import com.google.common.collect.ImmutableSet;
1921
import com.google.errorprone.BugPattern;
2022
import com.google.errorprone.BugPattern.SeverityLevel;
@@ -399,4 +401,29 @@ record Foo(String first, String second) {}
399401
""")
400402
.doTest();
401403
}
404+
405+
public record Foo(String first, String second) {}
406+
407+
@Test
408+
public void recordPattern() {
409+
assume().that(Runtime.version().feature()).isAtLeast(21);
410+
testHelper
411+
.addSourceLines(
412+
"Test.java",
413+
"""
414+
import %s;
415+
class Test {
416+
String test(Object o) {
417+
return switch (o) {
418+
case Foo(String first, String second) -> first;
419+
default -> "";
420+
};
421+
}
422+
}
423+
424+
"""
425+
.formatted(Foo.class.getCanonicalName()))
426+
.setArgs("--enable-preview", "--release", Integer.toString(Runtime.version().feature()))
427+
.doTest();
428+
}
402429
}

0 commit comments

Comments
 (0)