From 0a86d7103a6f4a9a9fcb1b9b5624445b69bec96c Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Sun, 30 Mar 2025 17:00:18 +0200 Subject: [PATCH] Java: detect more problems in annotations.xml (IJ-CR-158282) GitOrigin-RevId: 83cdba5a1cc7294ddd13191004d09e5d0d138343 --- .../BaseExternalAnnotationsManager.java | 17 +++-- .../ExternalAnnotationsManagerTest.java | 69 ++++++++++++++++++- .../java/nio/file/annotations.xml | 1 - 3 files changed, 75 insertions(+), 12 deletions(-) diff --git a/java/java-psi-impl/src/com/intellij/codeInsight/BaseExternalAnnotationsManager.java b/java/java-psi-impl/src/com/intellij/codeInsight/BaseExternalAnnotationsManager.java index a677787ad408..dbeff419d640 100644 --- a/java/java-psi-impl/src/com/intellij/codeInsight/BaseExternalAnnotationsManager.java +++ b/java/java-psi-impl/src/com/intellij/codeInsight/BaseExternalAnnotationsManager.java @@ -1,8 +1,9 @@ -// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +// Copyright 2000-2025 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package com.intellij.codeInsight; import com.intellij.lang.java.parser.JavaParser; import com.intellij.lang.java.parser.JavaParserUtil; +import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.util.Key; import com.intellij.openapi.util.LowMemoryWatcher; @@ -234,7 +235,7 @@ public abstract class BaseExternalAnnotationsManager extends ExternalAnnotations saxParser.parse(new InputSource(new CharSequenceReader(escapeAttributes(fileText))), handler); } catch (SAXParseException e) { - if (externalAnnotationsManager != null) { + if (externalAnnotationsManager != null && !ApplicationManager.getApplication().isUnitTestMode()) { externalAnnotationsManager.reportXmlParseError(virtualFile, e); } else { LOG.error(virtualFile.getPath(), e); @@ -568,6 +569,9 @@ public abstract class BaseExternalAnnotationsManager extends ExternalAnnotations } myArguments.append(attributes.getValue("val")); } + else if (!"root".equals(qName)) { + LOG.error("Unknown element name: " + qName + " in " + myFile.getPath()); + } } @Override @@ -581,13 +585,8 @@ public abstract class BaseExternalAnnotationsManager extends ExternalAnnotations if (existingData.annotationClassFqName.equals(myAnnotationFqn) && Objects.equals(myTypePath, existingData.typePath) && myExternalAnnotationsManager != null) { - myExternalAnnotationsManager.duplicateError(myFile, myExternalName, "Duplicate annotation '" + - myAnnotationFqn + - "'" - + - (myTypePath == null - ? "" - : " for type path '" + myTypePath + "'")); + String error = "Duplicate annotation '" + myAnnotationFqn + "'" + (myTypePath == null ? "" : " for type path '" + myTypePath + "'"); + myExternalAnnotationsManager.duplicateError(myFile, myExternalName, error); } } diff --git a/java/java-tests/testSrc/com/intellij/java/codeInsight/ExternalAnnotationsManagerTest.java b/java/java-tests/testSrc/com/intellij/java/codeInsight/ExternalAnnotationsManagerTest.java index 3ab25ed061a2..8b98762b0d93 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInsight/ExternalAnnotationsManagerTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInsight/ExternalAnnotationsManagerTest.java @@ -1,4 +1,4 @@ -// Copyright 2000-2022 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +// Copyright 2000-2025 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package com.intellij.java.codeInsight; import com.intellij.codeInsight.BaseExternalAnnotationsManager; @@ -20,6 +20,7 @@ import com.intellij.openapi.util.text.StringUtil; import com.intellij.openapi.vfs.*; import com.intellij.psi.*; import com.intellij.psi.search.GlobalSearchScope; +import com.intellij.psi.util.InheritanceUtil; import com.intellij.psi.util.PsiFormatUtil; import com.intellij.testFramework.LightPlatformTestCase; import com.intellij.testFramework.LightProjectDescriptor; @@ -29,6 +30,8 @@ import com.intellij.testFramework.fixtures.MavenDependencyUtil; import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.MostlySingularMultiMap; import com.intellij.xml.util.XmlUtil; +import com.siyeh.ig.psiutils.ClassUtils; +import junit.framework.AssertionFailedError; import one.util.streamex.StreamEx; import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NotNull; @@ -42,7 +45,8 @@ import java.util.Set; public class ExternalAnnotationsManagerTest extends LightPlatformTestCase { private static final Set KNOWN_EXCEPTIONS = Set.of( - "java.util.stream.Stream generate(java.util.function.Supplier)" // replaced with Supplier in JDK11 + "java.util.stream.Stream generate(java.util.function.Supplier)", // replaced with Supplier in JDK11 + "java.nio.charset.Charset charset()" // in java.io.PrintStream since JDK18, but test runs on JDK17 ); private final DefaultLightProjectDescriptor myDescriptor = new DefaultLightProjectDescriptor() { @@ -127,10 +131,71 @@ public class ExternalAnnotationsManagerTest extends LightPlatformTestCase { fail("Invalid typePath: " + error, psiFile, externalName); } } + else { + if (listOwner != null) { + if ("org.intellij.lang.annotations.MagicConstant".equals(nameText)) { + String typeText = getType(listOwner).getCanonicalText(); + assertTrue(externalName, "int".equals(typeText) || "long".equals(typeText) || "java.lang.String".equals(typeText)); + } + else if ("org.jetbrains.annotations.Nullable".equals(nameText) + || "org.jetbrains.annotations.NotNull".equals(nameText) + || "org.jetbrains.annotations.UnknownNullability".equals(nameText) + || "org.intellij.lang.annotations.Flow".equals(nameText)) { + assertFalse(externalName, getType(listOwner) instanceof PsiPrimitiveType); + } + else if ("org.jetbrains.annotations.Contract".equals(nameText)) { + assertTrue(externalName, listOwner instanceof PsiMethod); + } + else if ("org.jetbrains.annotations.Range".equals(nameText)) { + assertTrue(externalName, ClassUtils.isIntegral(getType(listOwner))); + } + else if ("org.jetbrains.annotations.NonNls".equals(nameText) || "org.jetbrains.annotations.Nls".equals(nameText)) { + if (listOwner instanceof PsiClass + || listOwner instanceof PsiPackage + || "javax.swing.JComponent java.lang.Object getClientProperty(java.lang.Object) 0".equals(externalName) + || "javax.swing.ActionMap javax.swing.Action get(java.lang.Object) 0".equals(externalName) + || "javax.swing.ActionMap void put(java.lang.Object, javax.swing.Action) 0".equals(externalName) + || "javax.swing.JComponent void putClientProperty(java.lang.Object, java.lang.Object) 0".equals(externalName) + || "javax.swing.JComponent void putClientProperty(java.lang.Object, java.lang.Object) 1".equals(externalName) + || "javax.swing.InputMap void put(javax.swing.KeyStroke, java.lang.Object) 1".equals(externalName)) { + // seems a little suspicious/weird but let it pass + continue; + } + String typeText = getType(listOwner).getCanonicalText(); + assertTrue(externalName, "java.lang.String".equals(typeText) + || "java.lang.String...".equals(typeText) + || "java.lang.String[]".equals(typeText)); + } + else if ("org.jetbrains.annotations.PropertyKey".equals(nameText)) { + String typeText = getType(listOwner).getCanonicalText(); + assertEquals(externalName, "java.lang.String", typeText); + } + else if ("org.jetbrains.annotations.Unmodifiable".equals(nameText) + || "org.jetbrains.annotations.UnmodifiableView".equals(nameText)) { + PsiType type = getType(listOwner); + assertTrue(InheritanceUtil.isInheritor(type, "java.util.Collection") || InheritanceUtil.isInheritor(type, "java.util.Map")); + } + else { + fail(externalName + " " + nameText); + } + } + } } } } + private static @NotNull PsiType getType(@NotNull PsiModifierListOwner listOwner) { + if (listOwner instanceof PsiMethod m) { + return m.getReturnType(); + } + else if (listOwner instanceof PsiVariable f) { + return f.getType(); + } + else { + throw new AssertionFailedError("" + listOwner); + } + } + private static String validatePath(String pathString, PsiType type) { if (!pathString.startsWith("/")) { return "Must start with '/'"; diff --git a/java/jdkAnnotations/java/nio/file/annotations.xml b/java/jdkAnnotations/java/nio/file/annotations.xml index 7bd05de6f880..ac2759c1702c 100644 --- a/java/jdkAnnotations/java/nio/file/annotations.xml +++ b/java/jdkAnnotations/java/nio/file/annotations.xml @@ -1038,7 +1038,6 @@ -