Improve error reporting around overloading

biboudis · Feb 7, 2025 · b314dba · b314dba
1 parent 5dc30e7
commit b314dba
Show file tree

Hide file tree

Showing 8 changed files with 102 additions and 34 deletions.
diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
@@ -175,6 +175,8 @@ protected Attr(Context context) {
                              Feature.PATTERN_SWITCH.allowedInSource(source);
         allowUnconditionalPatternsInstanceOf =
                              Feature.UNCONDITIONAL_PATTERN_IN_INSTANCEOF.allowedInSource(source);
+        allowPatternDeclarations = preview.isEnabled() && Feature.PATTERN_DECLARATIONS.allowedInSource(source);
+
         sourceName = source.name;
         useBeforeDeclarationWarning = options.isSet("useBeforeDeclarationWarning");
 
@@ -204,6 +206,10 @@ protected Attr(Context context) {
      */
     private final boolean allowUnconditionalPatternsInstanceOf;
 
+    /** Are pattern declarations allowed
+     */
+    private final boolean allowPatternDeclarations;
+
     /**
      * Switch: warn about use of variable before declaration?
      * RFE: 6425594
@@ -4346,24 +4352,36 @@ public void visitRecordPattern(JCRecordPattern tree) {
         if (site.tsym.kind == Kind.TYP) {
             int nestedPatternCount = tree.nested.size();
 
-            // Resolve deconstructor call for pattern-use side
-            // If site refers to a record, then synthesize a MT/MethodSymbol with the signature of the implicitely declared pattern declaration
-            List<MethodSymbol> patternDeclarations = patternDeclarationCandidatesWithArity(site, nestedPatternCount);
+            List<MethodSymbol> candidates = candidatesWithArity(site, nestedPatternCount);
 
-            if (patternDeclarations.size() >= 1) {
+            if (candidates.size() >= 1) {
                 List<Type> notionalTypes = calculateNotionalTypes(tree);
-                Symbol resolvedPatternDeclaration = selectBestPatternDeclarationInScope(tree.pos(), site, patternDeclarations, notionalTypes);
-
-                if (resolvedPatternDeclaration != null && resolvedPatternDeclaration.kind != AMBIGUOUS) {
-                    nestedPatternsTargetTypes = types.memberType(site, resolvedPatternDeclaration).getBindingTypes();
-                    tree.patternDeclaration = (MethodSymbol) resolvedPatternDeclaration;
+                Symbol resolved =
+                        selectBestPatternDeclarationInScope(tree.pos(), site, candidates, notionalTypes);
+
+                if (resolved != null && resolved.kind != AMBIGUOUS) {
+                    nestedPatternsTargetTypes = types.memberType(site, resolved).getBindingTypes();
+                    tree.patternDeclaration = (MethodSymbol) resolved;
+                } else if (resolved != null && resolved.kind == AMBIGUOUS){
+                    log.error(tree.pos(), Errors.MatcherOverloadingAmbiguity(site.tsym));
+                } else {
+                    if (site.tsym instanceof ClassSymbol cs && cs.isRecord() || !allowPatternDeclarations) {
+                        log.error(tree.pos(), Errors.CantApplyDeconstructionPattern(site.tsym));
+                    } else {
+                        log.error(tree.pos(), Errors.NoCompatibleMatcherFound(site.tsym));
+                    }
+                }
+            } else {
+                if (site.tsym instanceof ClassSymbol cs && cs.isRecord() || !allowPatternDeclarations) {
+                    log.error(tree.pos(), Errors.CantApplyDeconstructionPattern(site.tsym));
+                } else {
+                    log.error(tree.pos(),
+                            Errors.NoCompatibleMatcherFoundArity(site.tsym, String.valueOf(nestedPatternCount)));
                 }
             }
-            // TODO: error for classes with non matching arity
         }
 
         if (nestedPatternsTargetTypes == null) {
-            log.error(tree.pos(), Errors.CantApplyDeconstructionPattern(site.tsym));
             nestedPatternsTargetTypes = Stream.generate(() -> types.createErrorType(tree.type))
                                 .limit(tree.nested.size())
                                 .collect(List.collector());
@@ -4456,14 +4474,6 @@ private Symbol selectBestPatternDeclarationInScope(DiagnosticPosition pos,
             }
         }
 
-        if (bestSoFar != null && bestSoFar.kind == AMBIGUOUS) {
-            log.error(pos,
-                    Errors.MatcherOverloadingAmbiguity);
-        } else if (bestSoFar == null) {
-            log.error(pos,
-                    Errors.NoCompatibleMatcherFound);
-        }
-
         return bestSoFar;
     }
 
@@ -4617,7 +4627,7 @@ private boolean isApplicable(List<Type> patternDeclarationBindingType, List<Type
      * @param nestedPatternCount the number of nested patterns
      * @return                   a list of MethodSymbols
      */
-    private List<MethodSymbol> patternDeclarationCandidatesWithArity(Type site, int nestedPatternCount) {
+    private List<MethodSymbol> candidatesWithArity(Type site, int nestedPatternCount) {
         var matchersIt = site.tsym.members()
                 .getSymbols(sym -> sym.isPattern() && sym.type.getBindingTypes().size() == nestedPatternCount)
                 .iterator();

diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties
@@ -467,11 +467,17 @@ compiler.err.concrete.inheritance.conflict=\
 compiler.err.default.allowed.in.intf.annotation.member=\
     default value only allowed in an annotation interface declaration
 
+# 0: symbol
 compiler.err.matcher.overloading.ambiguity=\
-    matcher overloading ambiguity
+    matcher overloading ambiguity for pattern {0}
 
+# 0: symbol
 compiler.err.no.compatible.matcher.found=\
-    no compatible matcher found
+    no compatible matcher found for pattern {0}
+
+# 0: symbol, 1: string
+compiler.err.no.compatible.matcher.found.arity=\
+    no compatible matcher found with arity {1} for pattern {0}
 
 # 0: symbol
 compiler.err.doesnt.exist=\

diff --git a/test/langtools/tools/javac/diags/examples/NoCompatibleMatcherFoundArity.java b/test/langtools/tools/javac/diags/examples/NoCompatibleMatcherFoundArity.java
@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+// key: compiler.err.no.compatible.matcher.found.arity
+// key: compiler.misc.feature.pattern.declarations
+// key: compiler.warn.preview.feature.use.plural
+// options: --enable-preview -source ${jdk.version} -Xlint:preview
+
+public class NoCompatibleMatcherFoundArity {
+    private static int test(D o) {
+        if (o instanceof D(String data, Integer out, Float f)) {
+            return out;
+        }
+        return -1;
+    }
+
+    public static class D {
+        public pattern D(Object v1, Float out) {
+            match D(new Object(), 10.0f);
+        }
+
+        public pattern D(Float out, Integer v1) {
+            match D(10.0f, 42);
+        }
+    }
+}
diff --git a/test/langtools/tools/javac/patterns/DeconstructionPatternErrors.out b/test/langtools/tools/javac/patterns/DeconstructionPatternErrors.out
@@ -2,11 +2,11 @@ DeconstructionPatternErrors.java:35:37: compiler.err.illegal.start.of.type
 DeconstructionPatternErrors.java:37:28: compiler.err.illegal.start.of.type
 DeconstructionPatternErrors.java:39:42: compiler.err.expected: ';'
 DeconstructionPatternErrors.java:39:43: compiler.err.not.stmt
-DeconstructionPatternErrors.java:15:26: compiler.err.no.compatible.matcher.found
+DeconstructionPatternErrors.java:15:26: compiler.err.cant.apply.deconstruction.pattern: DeconstructionPatternErrors.P3
 DeconstructionPatternErrors.java:16:29: compiler.err.instanceof.reifiable.not.safe: java.lang.Object, java.util.ArrayList<java.lang.Integer>
-DeconstructionPatternErrors.java:17:26: compiler.err.no.compatible.matcher.found
-DeconstructionPatternErrors.java:18:26: compiler.err.no.compatible.matcher.found
-DeconstructionPatternErrors.java:19:26: compiler.err.no.compatible.matcher.found
+DeconstructionPatternErrors.java:17:26: compiler.err.cant.apply.deconstruction.pattern: DeconstructionPatternErrors.P5
+DeconstructionPatternErrors.java:18:26: compiler.err.cant.apply.deconstruction.pattern: DeconstructionPatternErrors.P
+DeconstructionPatternErrors.java:19:26: compiler.err.cant.apply.deconstruction.pattern: DeconstructionPatternErrors.P5
 DeconstructionPatternErrors.java:20:26: compiler.err.cant.apply.deconstruction.pattern: DeconstructionPatternErrors.P2
 DeconstructionPatternErrors.java:21:26: compiler.err.cant.apply.deconstruction.pattern: DeconstructionPatternErrors.P2
 DeconstructionPatternErrors.java:22:26: compiler.err.cant.apply.deconstruction.pattern: DeconstructionPatternErrors.P
@@ -15,7 +15,7 @@ DeconstructionPatternErrors.java:24:26: compiler.err.cant.apply.deconstruction.p
 DeconstructionPatternErrors.java:24:36: compiler.err.cant.resolve.location: kindname.class, Unresolvable, , , (compiler.misc.location: kindname.class, DeconstructionPatternErrors, null)
 DeconstructionPatternErrors.java:25:13: compiler.err.instanceof.reifiable.not.safe: java.lang.Object, DeconstructionPatternErrors.GenRecord<java.lang.String>
 DeconstructionPatternErrors.java:26:29: compiler.err.instanceof.reifiable.not.safe: java.lang.Object, DeconstructionPatternErrors.GenRecord<java.lang.String>
-DeconstructionPatternErrors.java:27:26: compiler.err.no.compatible.matcher.found
+DeconstructionPatternErrors.java:27:26: compiler.err.cant.apply.deconstruction.pattern: DeconstructionPatternErrors.GenRecord
 DeconstructionPatternErrors.java:27:13: compiler.err.instanceof.reifiable.not.safe: java.lang.Object, DeconstructionPatternErrors.GenRecord<java.lang.String>
 DeconstructionPatternErrors.java:28:40: compiler.err.match.binding.exists
 DeconstructionPatternErrors.java:29:56: compiler.err.already.defined: kindname.variable, v1, kindname.method, meth()

diff --git a/test/langtools/tools/javac/patterns/PrimitivePatternsSwitchErrors.out b/test/langtools/tools/javac/patterns/PrimitivePatternsSwitchErrors.out
@@ -1,6 +1,6 @@
 PrimitivePatternsSwitchErrors.java:15:18: compiler.err.pattern.dominated
 PrimitivePatternsSwitchErrors.java:24:18: compiler.err.pattern.dominated
-PrimitivePatternsSwitchErrors.java:31:18: compiler.err.no.compatible.matcher.found
+PrimitivePatternsSwitchErrors.java:31:18: compiler.err.cant.apply.deconstruction.pattern: PrimitivePatternsSwitchErrors.R_int
 PrimitivePatternsSwitchErrors.java:70:18: compiler.err.pattern.dominated
 PrimitivePatternsSwitchErrors.java:78:18: compiler.err.pattern.dominated
 PrimitivePatternsSwitchErrors.java:84:18: compiler.err.prob.found.req: (compiler.misc.possible.loss.of.precision: long, byte)

diff --git a/test/langtools/tools/javac/patterns/declarations/OverloadedPatternDeclarationErrors.java b/test/langtools/tools/javac/patterns/declarations/OverloadedPatternDeclarationErrors.java
@@ -10,14 +10,19 @@ static class I1 implements I {}
     static class I2 implements I {}
 
     private static int test(D o) {
-        if (o instanceof D(String data, Integer out)) { // no compatible matcher found
+        if (o instanceof D(String data, Integer out)) {                        // matching arity exists, no applicable overload found
             return out;
         }
         return -1;
     }
 
     private static void test2(D o) {
-        if (o instanceof D(I data)) {                  // ambiguity
+        if (o instanceof D(I data)) {                                          // matching arity exists, multiple applicable exist, ambiguous
+        }
+    }
+
+    private static void test3(D o) {
+        if (o instanceof D(I data, Integer i1, Integer i2)) {                  // no matching arity matcher exists
         }
     }
 

diff --git a/test/langtools/tools/javac/patterns/declarations/OverloadedPatternDeclarationErrors.out b/test/langtools/tools/javac/patterns/declarations/OverloadedPatternDeclarationErrors.out
@@ -1,5 +1,6 @@
-OverloadedPatternDeclarationErrors.java:13:26: compiler.err.no.compatible.matcher.found
-OverloadedPatternDeclarationErrors.java:20:26: compiler.err.matcher.overloading.ambiguity
+OverloadedPatternDeclarationErrors.java:13:26: compiler.err.no.compatible.matcher.found: OverloadedPatternDeclarationErrors.D
+OverloadedPatternDeclarationErrors.java:20:26: compiler.err.matcher.overloading.ambiguity: OverloadedPatternDeclarationErrors.D
+OverloadedPatternDeclarationErrors.java:25:26: compiler.err.no.compatible.matcher.found.arity: OverloadedPatternDeclarationErrors.D, 3
 - compiler.note.preview.filename: OverloadedPatternDeclarationErrors.java, DEFAULT
 - compiler.note.preview.recompile
-2 errors
+3 errors
diff --git a/...angtools/tools/javac/patterns/declarations/OverloadedPatternDeclarationsNonTemplated.java b/...angtools/tools/javac/patterns/declarations/OverloadedPatternDeclarationsNonTemplated.java
@@ -207,7 +207,7 @@ pattern R(EF ef, Object o) {
 
         compileAndRun(base, source,
                 """
-                        Test.java:8:17: compiler.err.matcher.overloading.ambiguity
+                        Test.java:8:17: compiler.err.matcher.overloading.ambiguity: test.Test.R
                         - compiler.note.preview.filename: Test.java, DEFAULT
                         - compiler.note.preview.recompile
                         1 error