From aa65705ec2e203500efc046ffd8ff143073de291 Mon Sep 17 00:00:00 2001 From: Pavel Rappo Date: Tue, 19 Nov 2024 16:04:47 +0000 Subject: [PATCH 1/7] Initial commit obtained from a cherry pick/squash of a now superseded PR. --- .../javax/lang/model/util/Elements.java | 6 ++ .../model/util/elements/overrides/S.java | 52 ++++++++++++++ .../elements/overrides/TestOverrides.java | 68 +++++++++++++++++++ 3 files changed, 126 insertions(+) create mode 100644 test/langtools/tools/javac/processing/model/util/elements/overrides/S.java create mode 100644 test/langtools/tools/javac/processing/model/util/elements/overrides/TestOverrides.java diff --git a/src/java.compiler/share/classes/javax/lang/model/util/Elements.java b/src/java.compiler/share/classes/javax/lang/model/util/Elements.java index 7f0b493572e0d..35da7563b12b0 100644 --- a/src/java.compiler/share/classes/javax/lang/model/util/Elements.java +++ b/src/java.compiler/share/classes/javax/lang/model/util/Elements.java @@ -778,6 +778,12 @@ default TypeElement getOutermostTypeElement(Element e) { * elements.getTypeElement("I")); * } * + * @apiNote The notion of overriding, by itself, does not involve the + * method's return type, listed exceptions, or, to some extent, access + * modifiers. These are additional requirements checked by the compiler; + * see JLS {@jls 8.4.8.3} for details. An implementation may choose not + * to check the additional requirements under some conditions. + * * @param overrider the first method, possible overrider * @param overridden the second method, possibly being overridden * @param type the class or interface of which the first method is a member diff --git a/test/langtools/tools/javac/processing/model/util/elements/overrides/S.java b/test/langtools/tools/javac/processing/model/util/elements/overrides/S.java new file mode 100644 index 0000000000000..8ea2618b8253d --- /dev/null +++ b/test/langtools/tools/javac/processing/model/util/elements/overrides/S.java @@ -0,0 +1,52 @@ +/* + * Copyright (c) 2023, 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. + */ + +class S { + + public void m() { } +} + +class T1 extends S { + + protected void m() { } +} + +class T2 extends S { + + void m() { } +} + +class T3 extends S { + + private void m() { } +} + +class T4 extends S { + + public int m() { return 0; } +} + +class T5 extends S { + + public void m() throws Exception { } +} diff --git a/test/langtools/tools/javac/processing/model/util/elements/overrides/TestOverrides.java b/test/langtools/tools/javac/processing/model/util/elements/overrides/TestOverrides.java new file mode 100644 index 0000000000000..e8cd1dac49dbf --- /dev/null +++ b/test/langtools/tools/javac/processing/model/util/elements/overrides/TestOverrides.java @@ -0,0 +1,68 @@ +/* + * Copyright (c) 2023, 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. + */ + +/* + * @test + * @bug 8174840 + * @library /tools/javac/lib + * @build JavacTestingAbstractProcessor TestOverrides + * @compile -processor TestOverrides -proc:only S.java + */ + +import java.util.Set; +import javax.annotation.processing.RoundEnvironment; +import javax.lang.model.element.ExecutableElement; +import javax.lang.model.element.TypeElement; + +import static javax.lang.model.element.ElementKind.METHOD; + +/* + * This test models a few cases where Elements.overrides produces a false + * positive which warrants @apiNote. + */ +public class TestOverrides extends JavacTestingAbstractProcessor { + + @Override + public boolean process(Set annotations, RoundEnvironment round) { + if (!round.processingOver()) { + var sm = mIn(elements.getTypeElement("S")); + for (var subtypeName : new String[]{"T1", "T2", "T3", "T4", "T5"}) { + var t = elements.getTypeElement(subtypeName); + var tm = mIn(t); + if (!elements.overrides(tm, sm, t)) + messager.printError(String.format( + "%s does not override %s from %s", tm, sm, t.getQualifiedName())); + } + } + return true; + } + + private ExecutableElement mIn(TypeElement t) { + return t.getEnclosedElements().stream() + .filter(e -> e.getKind() == METHOD) + .filter(e -> e.getSimpleName().toString().equals("m")) + .map(e -> (ExecutableElement) e) + .findAny() + .get(); + } +} From 36e5a4780784c49d6e28faba370bdb5807ace2dd Mon Sep 17 00:00:00 2001 From: Nizar Benalla Date: Sun, 5 Jan 2025 22:35:11 +0100 Subject: [PATCH 2/7] (C) 2025 --- .../share/classes/javax/lang/model/util/Elements.java | 2 +- .../tools/javac/processing/model/util/elements/overrides/S.java | 2 +- .../processing/model/util/elements/overrides/TestOverrides.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/java.compiler/share/classes/javax/lang/model/util/Elements.java b/src/java.compiler/share/classes/javax/lang/model/util/Elements.java index 35da7563b12b0..76141e89415aa 100644 --- a/src/java.compiler/share/classes/javax/lang/model/util/Elements.java +++ b/src/java.compiler/share/classes/javax/lang/model/util/Elements.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2025, 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 diff --git a/test/langtools/tools/javac/processing/model/util/elements/overrides/S.java b/test/langtools/tools/javac/processing/model/util/elements/overrides/S.java index 8ea2618b8253d..328112cfc2dbe 100644 --- a/test/langtools/tools/javac/processing/model/util/elements/overrides/S.java +++ b/test/langtools/tools/javac/processing/model/util/elements/overrides/S.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2023, 2025, 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 diff --git a/test/langtools/tools/javac/processing/model/util/elements/overrides/TestOverrides.java b/test/langtools/tools/javac/processing/model/util/elements/overrides/TestOverrides.java index e8cd1dac49dbf..f17b86f6aa6ed 100644 --- a/test/langtools/tools/javac/processing/model/util/elements/overrides/TestOverrides.java +++ b/test/langtools/tools/javac/processing/model/util/elements/overrides/TestOverrides.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2023, 2025, 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 From fb11329616a52ddd27e50916cdcc6b8a21952fb2 Mon Sep 17 00:00:00 2001 From: Nizar Benalla Date: Sun, 5 Jan 2025 23:08:33 +0100 Subject: [PATCH 3/7] Respond to feedback. Using Jan's comment to not imply particular requirement on the implementation. --- .../share/classes/javax/lang/model/util/Elements.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/java.compiler/share/classes/javax/lang/model/util/Elements.java b/src/java.compiler/share/classes/javax/lang/model/util/Elements.java index 76141e89415aa..73b92beebcf52 100644 --- a/src/java.compiler/share/classes/javax/lang/model/util/Elements.java +++ b/src/java.compiler/share/classes/javax/lang/model/util/Elements.java @@ -778,11 +778,11 @@ default TypeElement getOutermostTypeElement(Element e) { * elements.getTypeElement("I")); * } * - * @apiNote The notion of overriding, by itself, does not involve the - * method's return type, listed exceptions, or, to some extent, access - * modifiers. These are additional requirements checked by the compiler; - * see JLS {@jls 8.4.8.3} for details. An implementation may choose not - * to check the additional requirements under some conditions. + * @apiNote This method implements the overrides relation as specified in JLS {@jls 8.4.8.1}. + * It may not implement the additional compile-time checks that Java compilers follow, + * specified in JLS {@jls 8.4.8.1} and {@jls 8.4.8.3}. In particular, the additional constraints + * on thrown types, return types and those constraints on method modifiers not directly + * bound to the overriding relation as such. * * @param overrider the first method, possible overrider * @param overridden the second method, possibly being overridden From 4f8877a577685c658f5f7a5668339ad154dd21ed Mon Sep 17 00:00:00 2001 From: Nizar Benalla Date: Mon, 6 Jan 2025 13:33:58 +0100 Subject: [PATCH 4/7] Respond to feedback around the wording. --- .../share/classes/javax/lang/model/util/Elements.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/java.compiler/share/classes/javax/lang/model/util/Elements.java b/src/java.compiler/share/classes/javax/lang/model/util/Elements.java index 73b92beebcf52..f5bb209714af1 100644 --- a/src/java.compiler/share/classes/javax/lang/model/util/Elements.java +++ b/src/java.compiler/share/classes/javax/lang/model/util/Elements.java @@ -781,8 +781,7 @@ default TypeElement getOutermostTypeElement(Element e) { * @apiNote This method implements the overrides relation as specified in JLS {@jls 8.4.8.1}. * It may not implement the additional compile-time checks that Java compilers follow, * specified in JLS {@jls 8.4.8.1} and {@jls 8.4.8.3}. In particular, the additional constraints - * on thrown types, return types and those constraints on method modifiers not directly - * bound to the overriding relation as such. + * on exception types, return types, and method modifiers do not affect the overriding relation. * * @param overrider the first method, possible overrider * @param overridden the second method, possibly being overridden From 3493320e4f00a6eae12afe9a6c594bb5d469ad25 Mon Sep 17 00:00:00 2001 From: Nizar Benalla Date: Thu, 23 Jan 2025 14:48:19 +0100 Subject: [PATCH 5/7] Update according to review comments. --- .../share/classes/javax/lang/model/util/Elements.java | 7 +++---- .../processing/model/util/elements/overrides/S.java | 11 +++++++++++ .../model/util/elements/overrides/TestOverrides.java | 6 +----- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/java.compiler/share/classes/javax/lang/model/util/Elements.java b/src/java.compiler/share/classes/javax/lang/model/util/Elements.java index f5bb209714af1..4564e66c91e63 100644 --- a/src/java.compiler/share/classes/javax/lang/model/util/Elements.java +++ b/src/java.compiler/share/classes/javax/lang/model/util/Elements.java @@ -778,10 +778,9 @@ default TypeElement getOutermostTypeElement(Element e) { * elements.getTypeElement("I")); * } * - * @apiNote This method implements the overrides relation as specified in JLS {@jls 8.4.8.1}. - * It may not implement the additional compile-time checks that Java compilers follow, - * specified in JLS {@jls 8.4.8.1} and {@jls 8.4.8.3}. In particular, the additional constraints - * on exception types, return types, and method modifiers do not affect the overriding relation. + * @apiNote It may not implement the additional compile-time checks on exception types, + * return types, and method modifiers specified in JLS {@jls 8.4.8.1} and {@jls 8.4.8.3}, + * although implementations of this method are allowed to implement these additional checks. * * @param overrider the first method, possible overrider * @param overridden the second method, possibly being overridden diff --git a/test/langtools/tools/javac/processing/model/util/elements/overrides/S.java b/test/langtools/tools/javac/processing/model/util/elements/overrides/S.java index 328112cfc2dbe..f6da3324acb33 100644 --- a/test/langtools/tools/javac/processing/model/util/elements/overrides/S.java +++ b/test/langtools/tools/javac/processing/model/util/elements/overrides/S.java @@ -21,31 +21,42 @@ * questions. */ +/* + * This file models a few cases where Elements.overrides produces a false + * positive which warrants @apiNote. + */ + +// S.java does not compile because it violates the JLS rules for overrides class S { public void m() { } } +// `protected` is a weaker modifier than `public` class T1 extends S { protected void m() { } } +// `package-private` is a weaker modifier than `public` class T2 extends S { void m() { } } +// `private` methods cannot override public method class T3 extends S { private void m() { } } +// return type int is not compatible with void class T4 extends S { public int m() { return 0; } } +// adding a checked exception violates the override rule class T5 extends S { public void m() throws Exception { } diff --git a/test/langtools/tools/javac/processing/model/util/elements/overrides/TestOverrides.java b/test/langtools/tools/javac/processing/model/util/elements/overrides/TestOverrides.java index f17b86f6aa6ed..ad9b70896323a 100644 --- a/test/langtools/tools/javac/processing/model/util/elements/overrides/TestOverrides.java +++ b/test/langtools/tools/javac/processing/model/util/elements/overrides/TestOverrides.java @@ -36,10 +36,6 @@ import static javax.lang.model.element.ElementKind.METHOD; -/* - * This test models a few cases where Elements.overrides produces a false - * positive which warrants @apiNote. - */ public class TestOverrides extends JavacTestingAbstractProcessor { @Override @@ -51,7 +47,7 @@ public boolean process(Set annotations, RoundEnvironment var tm = mIn(t); if (!elements.overrides(tm, sm, t)) messager.printError(String.format( - "%s does not override %s from %s", tm, sm, t.getQualifiedName())); + "%s does not override from %s method %s", tm, t.getQualifiedName(),sm)); } } return true; From 54d6890c8b667b285fe9e7d2d937e8e71be706d0 Mon Sep 17 00:00:00 2001 From: Nizar Benalla Date: Thu, 23 Jan 2025 14:53:10 +0100 Subject: [PATCH 6/7] whitespace --- .../processing/model/util/elements/overrides/TestOverrides.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/langtools/tools/javac/processing/model/util/elements/overrides/TestOverrides.java b/test/langtools/tools/javac/processing/model/util/elements/overrides/TestOverrides.java index ad9b70896323a..724dda1d4820d 100644 --- a/test/langtools/tools/javac/processing/model/util/elements/overrides/TestOverrides.java +++ b/test/langtools/tools/javac/processing/model/util/elements/overrides/TestOverrides.java @@ -47,7 +47,7 @@ public boolean process(Set annotations, RoundEnvironment var tm = mIn(t); if (!elements.overrides(tm, sm, t)) messager.printError(String.format( - "%s does not override from %s method %s", tm, t.getQualifiedName(),sm)); + "%s does not override from %s method %s", tm, t.getQualifiedName(), sm)); } } return true; From e21de7d42aa34994b64507660cbd368b50daedd6 Mon Sep 17 00:00:00 2001 From: Nizar Benalla Date: Thu, 27 Mar 2025 23:49:08 +0000 Subject: [PATCH 7/7] feedback: improve api note to convey more context --- .../share/classes/javax/lang/model/util/Elements.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/java.compiler/share/classes/javax/lang/model/util/Elements.java b/src/java.compiler/share/classes/javax/lang/model/util/Elements.java index 4564e66c91e63..5d97674c53d56 100644 --- a/src/java.compiler/share/classes/javax/lang/model/util/Elements.java +++ b/src/java.compiler/share/classes/javax/lang/model/util/Elements.java @@ -778,9 +778,11 @@ default TypeElement getOutermostTypeElement(Element e) { * elements.getTypeElement("I")); * } * - * @apiNote It may not implement the additional compile-time checks on exception types, - * return types, and method modifiers specified in JLS {@jls 8.4.8.1} and {@jls 8.4.8.3}, - * although implementations of this method are allowed to implement these additional checks. + * @apiNote This method examines the method's name, signature, subclass relationship, and accessibility + * in determining whether one method overrides another, as specified in JLS {@jls 8.4.8.1}. + * In addition, an implementation may have stricter checks including method modifiers, return types and + * exception types as described in JLS {@jls 8.4.8.1} and {@jls 8.4.8.3}. + * Note that such additional compile-time checks are not guaranteed and may vary between implementations. * * @param overrider the first method, possible overrider * @param overridden the second method, possibly being overridden