Correct checking for internal/experimental API usage

core-processor/src/main/java/io/micronaut/context/visitor/InternalApiTypeElementVisitor.java

@@ -15,19 +15,20 @@
  */
 package io.micronaut.context.visitor;
 
+import io.micronaut.core.annotation.AnnotationMetadata;
 import io.micronaut.core.annotation.Experimental;
 import io.micronaut.core.annotation.Internal;
 import io.micronaut.core.annotation.NonNull;
 import io.micronaut.core.util.StringUtils;
 import io.micronaut.inject.ast.ClassElement;
 import io.micronaut.inject.ast.ConstructorElement;
 import io.micronaut.inject.ast.Element;
-import io.micronaut.inject.ast.FieldElement;
-import io.micronaut.inject.ast.MemberElement;
 import io.micronaut.inject.ast.MethodElement;
 import io.micronaut.inject.visitor.TypeElementVisitor;
 import io.micronaut.inject.visitor.VisitorContext;
 
+import java.util.Set;
+
 /**
  * Logs warnings during compilation if any class extends an internal or
  * experimental Micronaut API.
@@ -42,7 +43,17 @@ public class InternalApiTypeElementVisitor implements TypeElementVisitor<Object,
 
     private static final String MICRONAUT_PROCESSING_INTERNAL_WARNINGS = "micronaut.processing.internal.warnings";
 
-    private boolean warned = false;
+    private boolean isMicronautClass;
+    private boolean hasMicronautSuperClass;
+    private boolean warned;
+
+    @Override
+    public Set<String> getSupportedAnnotationNames() {
+        return Set.of(
+            Internal.class.getName(),
+            Experimental.class.getName()
+        );
+    }
 
     @NonNull
     @Override
@@ -52,46 +63,80 @@ public VisitorKind getVisitorKind() {
 
     @Override
     public void visitClass(ClassElement element, VisitorContext context) {
-        if (!element.getName().startsWith(IO_MICRONAUT)) {
-            warn(element, context);
+        reset();
+        isMicronautClass = isMicronautElement(element);
+        if (isMicronautClass) {
+            return;
+        }
+        ClassElement currentElement = element;
+        while (true) {
+            currentElement = currentElement.getSuperType().orElse(null);
+            if (currentElement == null) {
+                hasMicronautSuperClass = false;
+                break;
+            }
+            if (isMicronautElement(currentElement)) {
+                hasMicronautSuperClass = true;
+                if (isInternalOrExperimental(currentElement)) {
+                    warn(element, context);
+                }
+                break;
+            }
         }
     }
 
-    @Override
-    public void visitMethod(MethodElement element, VisitorContext context) {
-        warnMember(element, context);
+    private void reset() {
+        warned = false;
+        hasMicronautSuperClass = false;
+        isMicronautClass = false;
+    }
+
+    private boolean isMicronautElement(ClassElement element) {
+        return element.getName().startsWith(IO_MICRONAUT);
     }
 
     @Override
-    public void visitConstructor(ConstructorElement element, VisitorContext context) {
+    public void visitMethod(MethodElement element, VisitorContext context) {
         warnMember(element, context);
     }
 
     @Override
-    public void visitField(FieldElement element, VisitorContext context) {
+    public void visitConstructor(ConstructorElement element, VisitorContext context) {
         warnMember(element, context);
     }
 
-    private void warnMember(MemberElement element, VisitorContext context) {
-        if (!element.getOwningType().getName().startsWith(IO_MICRONAUT)) {
-            warn(element, context);
+    private void warnMember(MethodElement element, VisitorContext context) {
+        if (isMicronautClass || !hasMicronautSuperClass) {
+            return;
         }
+        if (!element.getDeclaringType().equals(element.getOwningType())) {
+            // We are only interested in declared methods
+            return;
+        }
+        if (!isInternalOrExperimental(element.getMethodAnnotationMetadata())) {
+            return;
+        }
+        // We can probably check if the method is actually overridden but let's avoid it for perf reasons
+        warn(element, context);
     }
 
     private void warn(Element element, VisitorContext context) {
-        if (element.hasAnnotation(Internal.class) || element.hasAnnotation(Experimental.class)) {
-            warned = true;
-            if (warnEnabled(context)) {
-                context.warn("Element extends or implements an internal or experimental Micronaut API", element);
-            }
+        warned = true;
+        if (warnEnabled(context)) {
+            context.warn("Element extends or implements an internal or experimental Micronaut API", element);
         }
     }
 
+    private boolean isInternalOrExperimental(AnnotationMetadata annotationMetadata) {
+        return annotationMetadata.hasAnnotation(Internal.class) || annotationMetadata.hasAnnotation(Experimental.class);
+    }
+
     @Override
     public void finish(VisitorContext visitorContext) {
         if (warned && warnEnabled(visitorContext)) {
             visitorContext.warn("Overriding an internal Micronaut API may result in breaking changes in minor or patch versions of the framework. Proceed with caution!", null);
         }
+        reset();
     }
 
     private boolean warnEnabled(VisitorContext visitorContext) {

inject-java-test/src/main/java/io/micronaut/annotation/processing/test/JavaParser.java

@@ -67,6 +67,13 @@ public JavaParser() {
                         compiler.getStandardFileManager(diagnosticCollector, Locale.getDefault(), UTF_8));
     }
 
+    /**
+     * @return The diagnostic collector
+     */
+    public DiagnosticCollector<JavaFileObject> getDiagnosticCollector() {
+        return diagnosticCollector;
+    }
+
     /**
      * @return The compiler used
      */

inject-java/src/test/groovy/io/micronaut/visitors/AbstractInternalClass.java

@@ -0,0 +1,12 @@
+package io.micronaut.visitors;
+
+import io.micronaut.core.annotation.Internal;
+
+@Internal
+public abstract class AbstractInternalClass {
+
+    public String foo() {
+        return "foo";
+    }
+
+}

inject-java/src/test/groovy/io/micronaut/visitors/AbstractInternalMethodClass.java

@@ -0,0 +1,12 @@
+package io.micronaut.visitors;
+
+import io.micronaut.core.annotation.Internal;
+
+public abstract class AbstractInternalMethodClass {
+
+    @Internal
+    public String foo() {
+        return "foo";
+    }
+
+}

inject-java/src/test/groovy/io/micronaut/visitors/InternalVisitor1Spec.groovy

@@ -0,0 +1,59 @@
+/*
+ * Copyright 2017-2024 original 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 io.micronaut.visitors
+
+import io.micronaut.annotation.processing.test.AbstractTypeElementSpec
+import io.micronaut.annotation.processing.test.JavaParser
+import spock.lang.Shared
+
+class InternalVisitor1Spec extends AbstractTypeElementSpec {
+
+    @Shared
+    def parser = new JavaParser()
+
+    void "test warning"() {
+        when:
+            def bd = buildBeanDefinition('test.TestController', '''
+package test;
+
+import io.micronaut.http.annotation.*;
+
+@Controller("/test")
+class TestController extends io.micronaut.visitors.AbstractInternalMethodClass {
+
+    @Get("/getMethod")
+    public String getMethod(String argument) {
+        return "";
+    }
+
+    @Post("/postMethod")
+    public String postMethod() {
+        return "";
+    }
+
+}
+''')
+        then:
+            bd
+            parser.diagnosticCollector.diagnostics.isEmpty()
+
+    }
+
+    @Override
+    protected JavaParser newJavaParser() {
+        return parser
+    }
+}

inject-java/src/test/groovy/io/micronaut/visitors/InternalVisitor2Spec.groovy

@@ -0,0 +1,66 @@
+/*
+ * Copyright 2017-2024 original 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 io.micronaut.visitors
+
+import io.micronaut.annotation.processing.test.AbstractTypeElementSpec
+import io.micronaut.annotation.processing.test.JavaParser
+import spock.lang.Shared
+
+class InternalVisitor2Spec extends AbstractTypeElementSpec {
+
+    @Shared
+    def parser = new JavaParser()
+
+    void "test warning"() {
+        when:
+            def bd = buildBeanDefinition('test.TestController', '''
+package test;
+
+import io.micronaut.http.annotation.*;
+
+@Controller("/test")
+class TestController extends io.micronaut.visitors.AbstractInternalMethodClass {
+
+    @Get("/getMethod")
+    public String getMethod(String argument) {
+        return "";
+    }
+
+    @Post("/postMethod")
+    public String postMethod() {
+        return "";
+    }
+
+    @Override
+    public String foo() {
+        return super.foo();
+    }
+
+}
+''')
+            def diagnostics = parser.diagnosticCollector.diagnostics
+        then:
+            bd
+            diagnostics.size() == 2
+            diagnostics[0].toString().contains("test/TestController.java:20: warning: Element extends or implements an internal or experimental Micronaut API")
+            diagnostics[1].toString().contains("warning: Overriding an internal Micronaut API may result in breaking changes in minor or patch versions of the framework. Proceed with caution!")
+    }
+
+    @Override
+    protected JavaParser newJavaParser() {
+        return parser
+    }
+}

inject-java/src/test/groovy/io/micronaut/visitors/InternalVisitor3Spec.groovy

@@ -0,0 +1,63 @@
+/*
+ * Copyright 2017-2024 original 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 io.micronaut.visitors
+
+import io.micronaut.annotation.processing.test.AbstractTypeElementSpec
+import io.micronaut.annotation.processing.test.JavaParser
+import spock.lang.Shared
+
+class InternalVisitor3Spec extends AbstractTypeElementSpec {
+
+    @Shared
+    def parser = new JavaParser()
+
+    void "test warning"() {
+        when:
+            def bd = buildBeanDefinition('test.TestController', '''
+package test;
+
+import io.micronaut.http.annotation.*;
+
+@Controller("/test")
+class TestController extends io.micronaut.visitors.AbstractInternalClass {
+
+    @Get("/getMethod")
+    public String getMethod(String argument) {
+        return "";
+    }
+
+    @Post("/postMethod")
+    public String postMethod() {
+        return "";
+    }
+
+}
+''')
+            def diagnostics = parser.diagnosticCollector.diagnostics
+        then:
+            bd
+            diagnostics.size() == 2
+            diagnostics[0].toString().contains("test/TestController.java:7: warning: Element extends or implements an internal or experimental Micronaut API")
+            diagnostics[1].toString().contains("warning: Overriding an internal Micronaut API may result in breaking changes in minor or patch versions of the framework. Proceed with caution!")
+
+
+    }
+
+    @Override
+    protected JavaParser newJavaParser() {
+        return parser
+    }
+}