From 94a666a0dfaa005602c910957c968def97d828a0 Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Sat, 12 Nov 2016 14:47:36 +0300 Subject: [PATCH] annotations.xml syntax checker --- .../BaseExternalAnnotationsManager.java | 12 +- .../ExternalAnnotationsManagerTest.java | 186 ++++++++++++++++++ java/jdkAnnotations/java/awt/annotations.xml | 3 - .../java/awt/datatransfer/annotations.xml | 2 +- .../java/awt/event/annotations.xml | 2 +- java/jdkAnnotations/java/lang/annotations.xml | 9 - .../java/security/annotations.xml | 2 +- java/jdkAnnotations/java/sql/annotations.xml | 2 +- java/jdkAnnotations/java/util/annotations.xml | 15 -- .../javax/swing/annotations.xml | 20 +- .../javax/swing/plaf/basic/annotations.xml | 2 +- java/jdkAnnotations/org/jdom/annotations.xml | 6 +- .../intellij/testFramework/PsiTestUtil.java | 12 +- 13 files changed, 221 insertions(+), 52 deletions(-) create mode 100644 java/java-tests/testSrc/com/intellij/codeInsight/ExternalAnnotationsManagerTest.java 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 fb18c6f4652c..836a222dcdf5 100644 --- a/java/java-psi-impl/src/com/intellij/codeInsight/BaseExternalAnnotationsManager.java +++ b/java/java-psi-impl/src/com/intellij/codeInsight/BaseExternalAnnotationsManager.java @@ -158,7 +158,7 @@ public abstract class BaseExternalAnnotationsManager extends ExternalAnnotations } @NotNull - private MostlySingularMultiMap getDataFromFile(@NotNull PsiFile file) { + MostlySingularMultiMap getDataFromFile(@NotNull PsiFile file) { Pair, Long> cached = myAnnotationFileToDataAndModStampCache.get(file); long fileModificationStamp = file.getModificationStamp(); if (cached != null && cached.getSecond() == fileModificationStamp) { @@ -336,16 +336,16 @@ public abstract class BaseExternalAnnotationsManager extends ExternalAnnotations throw new UnsupportedOperationException(); } - protected void cacheExternalAnnotations(@SuppressWarnings("UnusedParameters") @NotNull String packageName, - @NotNull PsiFile fromFile, - @NotNull List annotationFiles) { + void cacheExternalAnnotations(@SuppressWarnings("UnusedParameters") @NotNull String packageName, + @NotNull PsiFile fromFile, + @NotNull List annotationFiles) { VirtualFile virtualFile = fromFile.getVirtualFile(); if (virtualFile != null) { myExternalAnnotationsCache.put(virtualFile, annotationFiles); } } - private static class AnnotationData { + static class AnnotationData { private final String annotationClassFqName; private final String annotationParameters; @@ -357,7 +357,7 @@ public abstract class BaseExternalAnnotationsManager extends ExternalAnnotations } @NotNull - private PsiAnnotation getAnnotation(@NotNull BaseExternalAnnotationsManager context) { + PsiAnnotation getAnnotation(@NotNull BaseExternalAnnotationsManager context) { PsiAnnotation a = myAnnotation; if (a == null) { String text = "@" + annotationClassFqName + (annotationParameters.isEmpty() ? "" : "(" + annotationParameters + ")"); diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/ExternalAnnotationsManagerTest.java b/java/java-tests/testSrc/com/intellij/codeInsight/ExternalAnnotationsManagerTest.java new file mode 100644 index 000000000000..e3da0c5bd6a4 --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/codeInsight/ExternalAnnotationsManagerTest.java @@ -0,0 +1,186 @@ +/* + * Copyright 2000-2016 JetBrains s.r.o. + * + * 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 com.intellij.codeInsight; + +import com.intellij.openapi.application.ApplicationManager; +import com.intellij.openapi.application.PathManager; +import com.intellij.openapi.application.ex.PathManagerEx; +import com.intellij.openapi.projectRoots.Sdk; +import com.intellij.openapi.projectRoots.impl.JavaAwareProjectJdkTableImpl; +import com.intellij.openapi.roots.OrderRootType; +import com.intellij.openapi.roots.ProjectRootManager; +import com.intellij.openapi.util.io.FileUtil; +import com.intellij.openapi.util.text.StringUtil; +import com.intellij.openapi.vfs.*; +import com.intellij.openapi.vfs.newvfs.impl.VfsRootAccess; +import com.intellij.psi.*; +import com.intellij.psi.search.GlobalSearchScope; +import com.intellij.psi.util.PsiFormatUtil; +import com.intellij.testFramework.IdeaTestCase; +import com.intellij.testFramework.PsiTestUtil; +import com.intellij.util.containers.ContainerUtil; +import com.intellij.util.containers.MostlySingularMultiMap; +import com.intellij.xml.util.XmlUtil; +import org.jetbrains.annotations.NotNull; + +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.stream.Collectors; + +public class ExternalAnnotationsManagerTest extends IdeaTestCase { + @Override + protected Sdk getTestProjectJdk() { + Sdk jdk = JavaAwareProjectJdkTableImpl.getInstanceEx().getInternalJdk(); + Sdk sdk = PsiTestUtil.addJdkAnnotations(jdk); + String home = jdk.getHomeDirectory().getParent().getPath(); + String toolsPath = home + "/lib/tools.jar!/"; + VfsRootAccess.allowRootAccess(getTestRootDisposable(), home); + VirtualFile toolsJar = JarFileSystem.getInstance().findFileByPath(toolsPath); + + Sdk plusTools = PsiTestUtil.addRootsToJdk(sdk, OrderRootType.CLASSES, toolsJar); + + Collection utilClassPath = PathManager.getUtilClassPath(); + VirtualFile[] files = utilClassPath.stream() + .map(path -> path.endsWith(".jar") ? + JarFileSystem.getInstance() .findFileByPath(FileUtil.toSystemIndependentName(path) + "!/") : + LocalFileSystem.getInstance() .findFileByPath(FileUtil.toSystemIndependentName(path))) + .toArray(VirtualFile[]::new); + + Sdk result = PsiTestUtil.addRootsToJdk(plusTools, OrderRootType.CLASSES, files); + return result; + } + + public void testBundledAnnotationXmls() { + String root = PathManagerEx.getCommunityHomePath() + "/java/jdkAnnotations"; + findAnnotationsXmlAndCheck(root); + } + + private void findAnnotationsXmlAndCheck(String root) { + VirtualFile jdkAnnoRoot = LocalFileSystem.getInstance().findFileByPath(root); + VfsUtilCore.visitChildrenRecursively(jdkAnnoRoot, new VirtualFileVisitor() { + @Override + public boolean visitFile(@NotNull VirtualFile file) { + if (file.getName().equals("annotations.xml")) { + check(file); + } + return true; + } + }); + } + + // some android classes are missing in IDEA, e.g. android.support.annotation.NonNull + public void _testAndroidAnnotationsXml() { + VirtualFile lib = LocalFileSystem.getInstance().findFileByPath(PathManagerEx.getCommunityHomePath() + "/android/android/lib"); + VirtualFile[] androidJars = Arrays.stream(lib.getChildren()) + .map(file -> file.getName().endsWith(".jar") ? + JarFileSystem.getInstance().getJarRootForLocalFile(file) : + file) + .toArray(VirtualFile[]::new); + + ApplicationManager.getApplication().runWriteAction(() -> ProjectRootManager.getInstance(getProject()) + .setProjectSdk(PsiTestUtil.addRootsToJdk(getTestProjectJdk(), OrderRootType.CLASSES, androidJars))); + + String root = PathManagerEx.getCommunityHomePath() + "/android/android/annotations"; + findAnnotationsXmlAndCheck(root); + } + + private void check(VirtualFile file) { + //System.out.println("file = " + file); + ExternalAnnotationsManagerImpl manager = (ExternalAnnotationsManagerImpl)ExternalAnnotationsManager.getInstance(getProject()); + PsiFile psiFile = getPsiManager().findFile(file); + MostlySingularMultiMap map = manager.getDataFromFile(psiFile); + for (String externalName : map.keySet()) { + checkExternalName(psiFile, externalName); + + // 'annotation name="org.jetbrains.annotations.NotNull"' should have FQN + for (BaseExternalAnnotationsManager.AnnotationData annotationData : map.get(externalName)) { + PsiAnnotation annotation = annotationData.getAnnotation(manager); + String nameText = annotation.getNameReferenceElement().getText(); + assertClassFqn(nameText, psiFile, externalName); + } + } + } + + private PsiClass assertClassFqn(String text, PsiFile psiFile, String externalName) { + if (!PsiNameHelper.getInstance(getProject()).isQualifiedName(text) || !text.contains(".")) { + fail("'" + text + "' doesn't seem like a FQN", psiFile, externalName); + } + + PsiClass aClass = JavaPsiFacade.getInstance(getProject()).findClass(text, GlobalSearchScope.allScope(getProject())); + if (aClass == null) { + fail("'" + text + "' doesn't resolve to a class", psiFile, externalName); + } + return aClass; + } + + private static void fail(String error, PsiFile psiFile, String externalName) { + int offset = psiFile.getText().indexOf(XmlUtil.escape(externalName)); + int line = PsiDocumentManager.getInstance(psiFile.getProject()).getDocument(psiFile).getLineNumber(offset); + fail(error + "\nFile: " + psiFile.getVirtualFile().getPath() + ":" + (line+1) + " (offset: "+offset+")"); + } + + private void checkExternalName(PsiFile psiFile, String externalName) { + // 'item name="java.lang.ClassLoader java.net.URL getResource(java.lang.String) 0"' should have all FQNs + String unescaped = StringUtil.unescapeXml(externalName); + List words = StringUtil.split(unescaped, " "); + String className = words.get(0); + PsiClass aClass = assertClassFqn(className, psiFile, externalName); + if (words.size() == 1) return; + + String rest = unescaped.substring(className.length() + " ".length()); + + if (rest.indexOf('(') == -1) { + // field + String field = StringUtil.trim(rest); + PsiField psiField = aClass.findFieldByName(field, false); + if (psiField == null) { + fail("Field '"+field+"' not found in class '"+aClass.getQualifiedName()+"'", psiFile, externalName); + } + return; + } + String methodName = ContainerUtil.getLastItem(StringUtil.getWordsIn(rest.substring(0, rest.indexOf('(')))); + + String methodSignature = rest.substring(0, rest.indexOf(')') + 1); + String methodExternalName = className + " " + methodSignature; + + List methods = Arrays.stream(aClass.getMethods()) + .filter(method -> methodExternalName.equals(PsiFormatUtil.getExternalName(method, false, Integer.MAX_VALUE))) + .collect(Collectors.toList()); + boolean found = !methods.isEmpty(); + if (!found) { + List candidates = Arrays.stream(aClass.findMethodsByName(methodName, false)) + .map(method -> XmlUtil.escape(PsiFormatUtil.getExternalName(method, false, Integer.MAX_VALUE))) + .collect(Collectors.toList()); + String additionalMsg = candidates.isEmpty() ? "" : "\nMaybe you have meant one of these methods instead:\n"+StringUtil.join(candidates, "\n")+"\n"; + fail("This method was not found in class '"+aClass.getQualifiedName()+"':\n"+"'"+methodSignature+"'"+additionalMsg, psiFile, externalName); + } + + String parameterNumberText = StringUtil.trim(rest.substring(rest.indexOf(')') + 1)); + if (parameterNumberText.isEmpty()) return; + + try { + int paramNumber = Integer.parseInt(parameterNumberText); + PsiMethod method = methods.get(0); + if (method.getParameterList().getParametersCount() <= paramNumber) { + fail("Parameter number '"+paramNumber+"' is too big for a method '"+methodSignature+"'", psiFile, externalName); + } + } + catch (NumberFormatException e) { + fail("Parameter number is not an integer: '"+parameterNumberText+"'", psiFile, externalName); + } + } +} diff --git a/java/jdkAnnotations/java/awt/annotations.xml b/java/jdkAnnotations/java/awt/annotations.xml index 50d1f16ca819..74edac03f2b4 100644 --- a/java/jdkAnnotations/java/awt/annotations.xml +++ b/java/jdkAnnotations/java/awt/annotations.xml @@ -65,9 +65,6 @@ - - - diff --git a/java/jdkAnnotations/java/awt/datatransfer/annotations.xml b/java/jdkAnnotations/java/awt/datatransfer/annotations.xml index 329dbd0b61bf..afc6ca191604 100644 --- a/java/jdkAnnotations/java/awt/datatransfer/annotations.xml +++ b/java/jdkAnnotations/java/awt/datatransfer/annotations.xml @@ -2,7 +2,7 @@ - + diff --git a/java/jdkAnnotations/java/awt/event/annotations.xml b/java/jdkAnnotations/java/awt/event/annotations.xml index 8808ee8ae40c..00a5129e2911 100644 --- a/java/jdkAnnotations/java/awt/event/annotations.xml +++ b/java/jdkAnnotations/java/awt/event/annotations.xml @@ -30,7 +30,7 @@ - + diff --git a/java/jdkAnnotations/java/lang/annotations.xml b/java/jdkAnnotations/java/lang/annotations.xml index 7ce0469d41b8..572588fb1356 100644 --- a/java/jdkAnnotations/java/lang/annotations.xml +++ b/java/jdkAnnotations/java/lang/annotations.xml @@ -37,12 +37,6 @@ - - - - - - @@ -70,9 +64,6 @@ - - - diff --git a/java/jdkAnnotations/java/security/annotations.xml b/java/jdkAnnotations/java/security/annotations.xml index 96f7fd9febac..21f4ccae9a49 100644 --- a/java/jdkAnnotations/java/security/annotations.xml +++ b/java/jdkAnnotations/java/security/annotations.xml @@ -1,5 +1,5 @@ - + diff --git a/java/jdkAnnotations/java/sql/annotations.xml b/java/jdkAnnotations/java/sql/annotations.xml index 59c93eb08f23..b9c1e4a91140 100644 --- a/java/jdkAnnotations/java/sql/annotations.xml +++ b/java/jdkAnnotations/java/sql/annotations.xml @@ -120,7 +120,7 @@ - + diff --git a/java/jdkAnnotations/java/util/annotations.xml b/java/jdkAnnotations/java/util/annotations.xml index cbadb8b58ede..9301024eb2ab 100644 --- a/java/jdkAnnotations/java/util/annotations.xml +++ b/java/jdkAnnotations/java/util/annotations.xml @@ -612,21 +612,12 @@ - - - - - - - - - @@ -981,12 +972,6 @@ - - - - - - diff --git a/java/jdkAnnotations/javax/swing/annotations.xml b/java/jdkAnnotations/javax/swing/annotations.xml index 6d21cc9622e2..2dd236b883b6 100644 --- a/java/jdkAnnotations/javax/swing/annotations.xml +++ b/java/jdkAnnotations/javax/swing/annotations.xml @@ -322,6 +322,16 @@ + + + + + + + + + + @@ -331,16 +341,6 @@ - - - - - - - - - - diff --git a/java/jdkAnnotations/javax/swing/plaf/basic/annotations.xml b/java/jdkAnnotations/javax/swing/plaf/basic/annotations.xml index 420b4dcf6c75..f51d4d8ca431 100644 --- a/java/jdkAnnotations/javax/swing/plaf/basic/annotations.xml +++ b/java/jdkAnnotations/javax/swing/plaf/basic/annotations.xml @@ -10,7 +10,7 @@ - + diff --git a/java/jdkAnnotations/org/jdom/annotations.xml b/java/jdkAnnotations/org/jdom/annotations.xml index 02739c14c0c0..2171c711208e 100644 --- a/java/jdkAnnotations/org/jdom/annotations.xml +++ b/java/jdkAnnotations/org/jdom/annotations.xml @@ -36,13 +36,13 @@ - + - + - + diff --git a/platform/testFramework/src/com/intellij/testFramework/PsiTestUtil.java b/platform/testFramework/src/com/intellij/testFramework/PsiTestUtil.java index 9a1404a94bc0..50f5c6b04007 100644 --- a/platform/testFramework/src/com/intellij/testFramework/PsiTestUtil.java +++ b/platform/testFramework/src/com/intellij/testFramework/PsiTestUtil.java @@ -373,6 +373,14 @@ public class PsiTestUtil { public static Sdk addJdkAnnotations(@NotNull Sdk sdk) { String path = FileUtil.toSystemIndependentName(PlatformTestUtil.getCommunityPath()) + "/java/jdkAnnotations"; VirtualFile root = LocalFileSystem.getInstance().findFileByPath(path); + return addRootsToJdk(sdk, AnnotationOrderRootType.getInstance(), root); + } + + @NotNull + @Contract(pure=true) + public static Sdk addRootsToJdk(@NotNull Sdk sdk, + @NotNull OrderRootType rootType, + @NotNull VirtualFile... roots) { Sdk clone; try { clone = (Sdk)sdk.clone(); @@ -381,7 +389,9 @@ public class PsiTestUtil { throw new RuntimeException(e); } SdkModificator sdkModificator = clone.getSdkModificator(); - sdkModificator.addRoot(root, AnnotationOrderRootType.getInstance()); + for (VirtualFile root : roots) { + sdkModificator.addRoot(root, rootType); + } sdkModificator.commitChanges(); return clone; }