From 58514e34f6b5b6d878b10c52f776bd7c960d7862 Mon Sep 17 00:00:00 2001 From: "Pavel V. Talanov" Date: Tue, 13 Jan 2015 15:44:35 +0300 Subject: [PATCH 01/12] Refactor and improve CoreJavaFileManagerTest Remove redundant tests testing bucks as the last symbol of inner class name Test bucks in other positions --- .../intellij/psi/CoreJavaFileManagerTest.java | 203 +++++++----------- 1 file changed, 77 insertions(+), 126 deletions(-) diff --git a/java/java-tests/testSrc/com/intellij/psi/CoreJavaFileManagerTest.java b/java/java-tests/testSrc/com/intellij/psi/CoreJavaFileManagerTest.java index 5c12affa5e72..824c8849f4e7 100644 --- a/java/java-tests/testSrc/com/intellij/psi/CoreJavaFileManagerTest.java +++ b/java/java-tests/testSrc/com/intellij/psi/CoreJavaFileManagerTest.java @@ -21,143 +21,94 @@ import com.intellij.openapi.vfs.VirtualFile; import com.intellij.psi.search.GlobalSearchScope; import com.intellij.testFramework.PsiTestCase; import com.intellij.testFramework.PsiTestUtil; - -import java.util.LinkedList; -import java.util.Queue; +import org.intellij.lang.annotations.Language; +import org.jetbrains.annotations.NotNull; public class CoreJavaFileManagerTest extends PsiTestCase { - private VirtualFile prepareClasses(String clazzName, String clazzData) throws Exception { + public void testCommon() throws Exception { + CoreJavaFileManager manager = configureManager("package foo;\n\n" + + "public class TopLevel {\n" + + "public class Inner {\n" + + " public class Inner {}\n" + + "}\n" + + "\n" + + "}"); + + assertCanFind(manager, "foo.TopLevel"); + assertCanFind(manager, "foo.TopLevel.Inner"); + assertCanFind(manager, "foo.TopLevel.Inner.Inner"); + } + + public void testInnerClassesWithDollars() throws Exception { + + CoreJavaFileManager manager = configureManager("package foo;\n\n" + + "public class TopLevel {\n" + + + "public class I$nner {" + + " public class I$nner{}" + + " public class $Inner{}" + + " public class In$ne$r${}" + + " public class Inner$${}" + + " public class $$$$${}" + + "}\n" + + "public class Inner$ {" + + " public class I$nner{}" + + " public class $Inner{}" + + " public class In$ne$r${}" + + " public class Inner$${}" + + " public class $$$$${}" + + "}\n" + + "public class In$ner$$ {" + + " public class I$nner{}" + + " public class $Inner{}" + + " public class In$ne$r${}" + + " public class Inner$${}" + + " public class $$$$${}" + + "}\n" + + "\n" + + "}"); + + assertCanFind(manager, "foo.TopLevel"); + + assertCanFind(manager, "foo.TopLevel.I$nner"); + assertCanFind(manager, "foo.TopLevel.I$nner.I$nner"); + assertCanFind(manager, "foo.TopLevel.I$nner.$Inner"); + assertCanFind(manager, "foo.TopLevel.I$nner.In$ne$r$"); + assertCanFind(manager, "foo.TopLevel.I$nner.Inner$$"); + assertCanFind(manager, "foo.TopLevel.I$nner.$$$$$"); + + assertCanFind(manager, "foo.TopLevel.Inner$"); + assertCanFind(manager, "foo.TopLevel.Inner$.I$nner"); + assertCanFind(manager, "foo.TopLevel.Inner$.$Inner"); + assertCanFind(manager, "foo.TopLevel.Inner$.In$ne$r$"); + assertCanFind(manager, "foo.TopLevel.Inner$.Inner$$"); + assertCanFind(manager, "foo.TopLevel.Inner$.$$$$$"); + + assertCanFind(manager, "foo.TopLevel.In$ner$$"); + assertCanFind(manager, "foo.TopLevel.In$ner$$.I$nner"); + assertCanFind(manager, "foo.TopLevel.In$ner$$.$Inner"); + assertCanFind(manager, "foo.TopLevel.In$ner$$.In$ne$r$"); + assertCanFind(manager, "foo.TopLevel.In$ner$$.Inner$$"); + assertCanFind(manager, "foo.TopLevel.In$ner$$.$$$$$"); + } + + @NotNull + private CoreJavaFileManager configureManager(@Language("JAVA") @NotNull String text) throws Exception { VirtualFile root = PsiTestUtil.createTestProjectStructure(myProject, myModule, myFilesToDelete); VirtualFile pkg = root.createChildDirectory(this, "foo"); PsiDirectory dir = myPsiManager.findDirectory(pkg); assertNotNull(dir); - dir.add(PsiFileFactory.getInstance(getProject()).createFileFromText(clazzName + ".java", JavaFileType.INSTANCE, clazzData)); - return root; - } - - public void testNotNullInnerClass() throws Exception { - String text = "package foo;\n\n" + - "public class Nested {\n" + - "public class InnerGeneral {}\n" + - "public class Inner$ {" + - "}\n" + - "\n" + - "public Inner$ inner() {\n" + - " return new Inner$();\n" + - "}\n" + - "\n" + - "}"; - - VirtualFile root = prepareClasses("Nested", text); - GlobalSearchScope scope = GlobalSearchScope.allScope(getProject()); + dir.add(PsiFileFactory.getInstance(getProject()).createFileFromText("TopLevel.java", JavaFileType.INSTANCE, text)); CoreJavaFileManager manager = new CoreJavaFileManager(myPsiManager); manager.addToClasspath(root); - - PsiClass clazz = manager.findClass("foo.Nested", scope); - assertNotNull(clazz); - - PsiClass clazzInnerGeneral = manager.findClass("foo.Nested.InnerGeneral", scope); - assertNotNull(clazzInnerGeneral); - - PsiClass clazzInner$ = manager.findClass("foo.Nested.Inner$", scope); - assertNotNull(clazzInner$); - - PsiClass clazzInner$Wrong1 = manager.findClass("foo.Nested.Inner$X", scope); - assertNull(clazzInner$Wrong1); - - PsiClass clazzInner$Wrong2 = manager.findClass("foo.Nested.Inner$$X", scope); - assertNull(clazzInner$Wrong2); - - PsiClass clazzInner$Wrong3 = manager.findClass("foo.Nested.Inner$$", scope); - assertNull(clazzInner$Wrong3); + return manager; } - - public void testNotNullInnerClass2() throws Exception { - String text = "package foo;\n\n" + - "public class Nested {\n" + - - "public class Inner {" + - " public class XInner{}" + - " public class XInner${}" + - "}\n" + - "public class Inner$ {" + - " public class XInner{}" + - " public class XInner${}" + - "}\n" + - "\n" + - "}"; - - VirtualFile root = prepareClasses("Nested", text); - GlobalSearchScope scope = GlobalSearchScope.allScope(getProject()); - CoreJavaFileManager manager = new CoreJavaFileManager(myPsiManager); - manager.addToClasspath(root); - - PsiClass clazzInner = manager.findClass("foo.Nested.Inner", scope); - assertNotNull(clazzInner); - - PsiClass clazzXInner = manager.findClass("foo.Nested.Inner.XInner", scope); - assertNotNull(clazzXInner); - - PsiClass clazzXInner$ = manager.findClass("foo.Nested.Inner.XInner$", scope); - assertNotNull(clazzXInner$); - - PsiClass clazz$XInner = manager.findClass("foo.Nested.Inner$.XInner", scope); - assertNotNull(clazz$XInner); - - PsiClass clazz$XInner$ = manager.findClass("foo.Nested.Inner$.XInner$", scope); - assertNotNull(clazz$XInner$); + private void assertCanFind(@NotNull CoreJavaFileManager manager, @NotNull String qName) { + PsiClass foundClass = manager.findClass(qName, GlobalSearchScope.allScope(getProject())); + assertNotNull("Could not find:" + qName, foundClass); + assertEquals("Found " + foundClass.getQualifiedName() + " instead of " + qName, qName, foundClass.getQualifiedName()); } - - - public void testNotNullInnerClass3() throws Exception { - String text = "package foo;\n\n" + - "public class NestedX {\n" + - - "public class XX {" + - " public class XXX{" + - " public class XXXX{ }" + - " public class XXXX${ }" + - " }" + - " public class XXX${" + - " public class XXXX{ }" + - " public class XXXX${ }" + - " }" + - "}\n" + - "public class XX$ {" + - " public class XXX{" + - " public class XXXX{ }" + - " public class XXXX${ }" + - " }" + - " public class XXX${" + - " public class XXXX{ }" + - " public class XXXX${ }" + - " }" + - "}\n" + - "\n" + - "}"; - - VirtualFile root = prepareClasses("NestedX", text); - GlobalSearchScope scope = GlobalSearchScope.allScope(getProject()); - CoreJavaFileManager manager = new CoreJavaFileManager(myPsiManager); - manager.addToClasspath(root); - - Queue queue = new LinkedList(); - queue.add("foo.NestedX"); - - while(!queue.isEmpty()) { - String head = queue.remove(); - PsiClass clazzInner = manager.findClass(head, scope); - assertNotNull(head, clazzInner); - String lastSegment = head.substring(head.lastIndexOf('.')); - String xs = lastSegment.substring(lastSegment.indexOf("X")).replace("$", ""); - if (xs.length() < 4) { - queue.add(head + "." + xs + "X"); - queue.add(head + "." + xs + "X$"); - } - } - } - } From 6dbe78cdff212170d16d4011e49defc997f35482 Mon Sep 17 00:00:00 2001 From: "Pavel V. Talanov" Date: Tue, 13 Jan 2015 16:27:34 +0300 Subject: [PATCH 02/12] Fix CoreJavaFileManager for top level classes with dollar in name --- .../intellij/core/CoreJavaFileManager.java | 124 +++++++----------- .../intellij/psi/CoreJavaFileManagerTest.java | 55 +++++++- 2 files changed, 97 insertions(+), 82 deletions(-) diff --git a/java/java-psi-impl/src/com/intellij/core/CoreJavaFileManager.java b/java/java-psi-impl/src/com/intellij/core/CoreJavaFileManager.java index 0cfb60b3ae69..4fb3ab0e8ebe 100644 --- a/java/java-psi-impl/src/com/intellij/core/CoreJavaFileManager.java +++ b/java/java-psi-impl/src/com/intellij/core/CoreJavaFileManager.java @@ -17,6 +17,7 @@ package com.intellij.core; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.util.io.FileUtil; +import com.intellij.openapi.util.text.StringUtil; import com.intellij.openapi.vfs.VfsUtilCore; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.psi.*; @@ -108,77 +109,59 @@ public class CoreJavaFileManager implements JavaFileManager { cur = child; } - String className = pathRest.replace('.', '$'); - int bucks = className.indexOf('$'); + String classNameWithInnerClasses = pathRest; + String topLevelClassName = substringBeforeFirstDot(classNameWithInnerClasses); - String rootClassName; - if (bucks < 0) { - rootClassName = className; + VirtualFile vFile = cur.findChild(topLevelClassName + ".class"); + if (vFile == null) vFile = cur.findChild(topLevelClassName + ".java"); + + if (vFile == null) { + return null; + } + if (!vFile.isValid()) { + LOG.error("Invalid child of valid parent: " + vFile.getPath() + "; " + root.isValid() + " path=" + root.getPath()); + return null; + } + + final PsiFile file = psiManager.findFile(vFile); + if (!(file instanceof PsiClassOwner)) { + return null; + } + + return findClassInPsiFile(classNameWithInnerClasses, (PsiClassOwner)file); + } + + @NotNull + private static String substringBeforeFirstDot(@NotNull String classNameWithInnerClasses) { + int dot = classNameWithInnerClasses.indexOf('.'); + if (dot < 0) { + return classNameWithInnerClasses; } else { - rootClassName = className.substring(0, bucks); - className = className.substring(bucks + 1); + return classNameWithInnerClasses.substring(0, dot); } + } - VirtualFile vFile = cur.findChild(rootClassName + ".class"); - if (vFile == null) vFile = cur.findChild(rootClassName + ".java"); - - if (vFile != null) { - if (!vFile.isValid()) { - LOG.error("Invalid child of valid parent: " + vFile.getPath() + "; " + root.isValid() + " path=" + root.getPath()); + @Nullable + private static PsiClass findClassInPsiFile(@NotNull String classNameWithInnerClassesDotSeparated, @NotNull PsiClassOwner file) { + final PsiClass[] classes = file.getClasses(); + if (classes.length != 1) { + return null; + } + PsiClass curClass = classes[0]; + Iterator segments = StringUtil.split(classNameWithInnerClassesDotSeparated, ".").iterator(); + if (!segments.hasNext() || !segments.next().equals(curClass.getName())) { + return null; + } + while (segments.hasNext()) { + String innerClassName = segments.next(); + PsiClass innerClass = curClass.findInnerClassByName(innerClassName, false); + if (innerClass == null) { return null; } - - final PsiFile file = psiManager.findFile(vFile); - if (file instanceof PsiClassOwner) { - final PsiClass[] classes = ((PsiClassOwner)file).getClasses(); - if (classes.length == 1) { - PsiClass curClass = classes[0]; - - if (bucks > 0) { - Stack currentPath = new Stack(); - currentPath.add(new ClassAndOffsets(curClass, 0, 0)); - currentPath.add(currentPath.peek()); - - while (currentPath.size() > 1) { - ClassAndOffsets classAndOffset = currentPath.pop(); - int newComponentStart = classAndOffset.componentStart; - int lookupStart = classAndOffset.lookupStart; - curClass = currentPath.peek().clazz; //owner class - - while (lookupStart <= className.length()) { - int bucksIndex = className.indexOf("$", lookupStart); - bucksIndex = bucksIndex < 0 ? className.length(): bucksIndex; - - String component = className.substring(newComponentStart, bucksIndex); - PsiClass inner = curClass.findInnerClassByName(component, false); - - lookupStart = bucksIndex + 1; - if (inner == null) { - continue; - } - - currentPath.add(new ClassAndOffsets(inner, newComponentStart, lookupStart)); - - newComponentStart = lookupStart; - curClass = inner; - } - - if (lookupStart == newComponentStart) { - return curClass; - } - } - - return null; - - } else { - return curClass; - } - } - } - } - - return null; + curClass = innerClass; + } + return curClass; } @NotNull @@ -203,17 +186,4 @@ public class CoreJavaFileManager implements JavaFileManager { public void addToClasspath(VirtualFile root) { myClasspath.add(root); } - - private static class ClassAndOffsets { - - final PsiClass clazz; - final int componentStart; - final int lookupStart; - - ClassAndOffsets(PsiClass clazz, int componentStart, int lookupStart) { - this.clazz = clazz; - this.componentStart = componentStart; - this.lookupStart = lookupStart; - } - } } diff --git a/java/java-tests/testSrc/com/intellij/psi/CoreJavaFileManagerTest.java b/java/java-tests/testSrc/com/intellij/psi/CoreJavaFileManagerTest.java index 824c8849f4e7..9bd8a899bec3 100644 --- a/java/java-tests/testSrc/com/intellij/psi/CoreJavaFileManagerTest.java +++ b/java/java-tests/testSrc/com/intellij/psi/CoreJavaFileManagerTest.java @@ -34,7 +34,7 @@ public class CoreJavaFileManagerTest extends PsiTestCase { " public class Inner {}\n" + "}\n" + "\n" + - "}"); + "}", "TopLevel"); assertCanFind(manager, "foo.TopLevel"); assertCanFind(manager, "foo.TopLevel.Inner"); @@ -42,7 +42,6 @@ public class CoreJavaFileManagerTest extends PsiTestCase { } public void testInnerClassesWithDollars() throws Exception { - CoreJavaFileManager manager = configureManager("package foo;\n\n" + "public class TopLevel {\n" + @@ -68,7 +67,7 @@ public class CoreJavaFileManagerTest extends PsiTestCase { " public class $$$$${}" + "}\n" + "\n" + - "}"); + "}", "TopLevel"); assertCanFind(manager, "foo.TopLevel"); @@ -94,13 +93,59 @@ public class CoreJavaFileManagerTest extends PsiTestCase { assertCanFind(manager, "foo.TopLevel.In$ner$$.$$$$$"); } + public void testTopLevelClassesWithDollars() throws Exception { + CoreJavaFileManager inTheMiddle = configureManager("package foo;\n\n public class Top$Level {}", "Top$Level"); + assertCanFind(inTheMiddle, "foo.Top$Level"); + + CoreJavaFileManager doubleAtTheEnd = configureManager("package foo;\n\n public class TopLevel$$ {}", "TopLevel$$"); + assertCanFind(doubleAtTheEnd, "foo.TopLevel$$"); + + CoreJavaFileManager multiple = configureManager("package foo;\n\n public class Top$Lev$el$ {}", "Top$Lev$el$"); + assertCanFind(multiple, "foo.Top$Lev$el$"); + + CoreJavaFileManager twoBucks = configureManager("package foo;\n\n public class $$ {}", "$$"); + assertCanFind(twoBucks, "foo.$$"); + } + + public void testTopLevelClassWithDollarsAndInners() throws Exception { + CoreJavaFileManager manager = configureManager("package foo;\n\n" + + "public class Top$Level$$ {\n" + + + "public class I$nner {" + + " public class I$nner{}" + + " public class In$ne$r${}" + + " public class Inner$$$$${}" + + " public class $Inner{}" + + " public class ${}" + + " public class $$$$${}" + + "}\n" + + "public class Inner {" + + " public class Inner{}" + + "}\n" + + "\n" + + "}", "Top$Level$$"); + + assertCanFind(manager, "foo.Top$Level$$"); + + assertCanFind(manager, "foo.Top$Level$$.Inner"); + assertCanFind(manager, "foo.Top$Level$$.Inner.Inner"); + + assertCanFind(manager, "foo.Top$Level$$.I$nner"); + assertCanFind(manager, "foo.Top$Level$$.I$nner.I$nner"); + assertCanFind(manager, "foo.Top$Level$$.I$nner.In$ne$r$"); + assertCanFind(manager, "foo.Top$Level$$.I$nner.Inner$$$$$"); + assertCanFind(manager, "foo.Top$Level$$.I$nner.$Inner"); + assertCanFind(manager, "foo.Top$Level$$.I$nner.$"); + assertCanFind(manager, "foo.Top$Level$$.I$nner.$$$$$"); + } + @NotNull - private CoreJavaFileManager configureManager(@Language("JAVA") @NotNull String text) throws Exception { + private CoreJavaFileManager configureManager(@Language("JAVA") @NotNull String text, @NotNull String className) throws Exception { VirtualFile root = PsiTestUtil.createTestProjectStructure(myProject, myModule, myFilesToDelete); VirtualFile pkg = root.createChildDirectory(this, "foo"); PsiDirectory dir = myPsiManager.findDirectory(pkg); assertNotNull(dir); - dir.add(PsiFileFactory.getInstance(getProject()).createFileFromText("TopLevel.java", JavaFileType.INSTANCE, text)); + dir.add(PsiFileFactory.getInstance(getProject()).createFileFromText(className + ".java", JavaFileType.INSTANCE, text)); CoreJavaFileManager manager = new CoreJavaFileManager(myPsiManager); manager.addToClasspath(root); return manager; From 15989988f699ac074c0edf0644702c4b11214186 Mon Sep 17 00:00:00 2001 From: "Pavel V. Talanov" Date: Tue, 13 Jan 2015 17:56:48 +0300 Subject: [PATCH 03/12] CoreJavaFileManagerTest: add negative scenarios --- .../intellij/psi/CoreJavaFileManagerTest.java | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/java/java-tests/testSrc/com/intellij/psi/CoreJavaFileManagerTest.java b/java/java-tests/testSrc/com/intellij/psi/CoreJavaFileManagerTest.java index 9bd8a899bec3..c44eed5977e1 100644 --- a/java/java-tests/testSrc/com/intellij/psi/CoreJavaFileManagerTest.java +++ b/java/java-tests/testSrc/com/intellij/psi/CoreJavaFileManagerTest.java @@ -39,6 +39,10 @@ public class CoreJavaFileManagerTest extends PsiTestCase { assertCanFind(manager, "foo.TopLevel"); assertCanFind(manager, "foo.TopLevel.Inner"); assertCanFind(manager, "foo.TopLevel.Inner.Inner"); + + assertCannotFind(manager, "foo.TopLevel$Inner.Inner"); + assertCannotFind(manager, "foo.TopLevel.Inner$Inner"); + assertCannotFind(manager, "foo.TopLevel.Inner.Inner.Inner"); } public void testInnerClassesWithDollars() throws Exception { @@ -78,6 +82,8 @@ public class CoreJavaFileManagerTest extends PsiTestCase { assertCanFind(manager, "foo.TopLevel.I$nner.Inner$$"); assertCanFind(manager, "foo.TopLevel.I$nner.$$$$$"); + assertCannotFind(manager, "foo.TopLevel.I.nner.$$$$$"); + assertCanFind(manager, "foo.TopLevel.Inner$"); assertCanFind(manager, "foo.TopLevel.Inner$.I$nner"); assertCanFind(manager, "foo.TopLevel.Inner$.$Inner"); @@ -85,12 +91,16 @@ public class CoreJavaFileManagerTest extends PsiTestCase { assertCanFind(manager, "foo.TopLevel.Inner$.Inner$$"); assertCanFind(manager, "foo.TopLevel.Inner$.$$$$$"); + assertCannotFind(manager, "foo.TopLevel.Inner..$$$$$"); + assertCanFind(manager, "foo.TopLevel.In$ner$$"); assertCanFind(manager, "foo.TopLevel.In$ner$$.I$nner"); assertCanFind(manager, "foo.TopLevel.In$ner$$.$Inner"); assertCanFind(manager, "foo.TopLevel.In$ner$$.In$ne$r$"); assertCanFind(manager, "foo.TopLevel.In$ner$$.Inner$$"); assertCanFind(manager, "foo.TopLevel.In$ner$$.$$$$$"); + + assertCannotFind(manager, "foo.TopLevel.In.ner$$.$$$$$"); } public void testTopLevelClassesWithDollars() throws Exception { @@ -102,6 +112,7 @@ public class CoreJavaFileManagerTest extends PsiTestCase { CoreJavaFileManager multiple = configureManager("package foo;\n\n public class Top$Lev$el$ {}", "Top$Lev$el$"); assertCanFind(multiple, "foo.Top$Lev$el$"); + assertCannotFind(multiple, "foo.Top.Lev$el$"); CoreJavaFileManager twoBucks = configureManager("package foo;\n\n public class $$ {}", "$$"); assertCanFind(twoBucks, "foo.$$"); @@ -137,6 +148,17 @@ public class CoreJavaFileManagerTest extends PsiTestCase { assertCanFind(manager, "foo.Top$Level$$.I$nner.$Inner"); assertCanFind(manager, "foo.Top$Level$$.I$nner.$"); assertCanFind(manager, "foo.Top$Level$$.I$nner.$$$$$"); + + assertCannotFind(manager, "foo.Top.Level$$.I$nner.$$$$$"); + } + + public void testDoNotThrowOnMalformedInput() throws Exception { + CoreJavaFileManager fileWithEmptyName = configureManager("package foo;\n\n public class Top$Level {}", ""); + assertCannotFind(fileWithEmptyName, "foo."); + assertCannotFind(fileWithEmptyName, "."); + assertCannotFind(fileWithEmptyName, ".."); + assertCannotFind(fileWithEmptyName, ""); + assertCannotFind(fileWithEmptyName, ".foo"); } @NotNull @@ -156,4 +178,9 @@ public class CoreJavaFileManagerTest extends PsiTestCase { assertNotNull("Could not find:" + qName, foundClass); assertEquals("Found " + foundClass.getQualifiedName() + " instead of " + qName, qName, foundClass.getQualifiedName()); } + + private void assertCannotFind(@NotNull CoreJavaFileManager manager, @NotNull String qName) { + PsiClass foundClass = manager.findClass(qName, GlobalSearchScope.allScope(getProject())); + assertNull("Found, but shouldn't have:" + qName, foundClass); + } } From 90e2339adfda6f865cc28cc904dcc9fb9a8cc538 Mon Sep 17 00:00:00 2001 From: "Pavel V. Talanov" Date: Tue, 13 Jan 2015 18:56:15 +0300 Subject: [PATCH 04/12] CoreJavaFileManager: find main class in file with several classes present It's unclear how to support finding other classes in such files --- .../com/intellij/core/CoreJavaFileManager.java | 17 ++++++++++++----- .../intellij/psi/CoreJavaFileManagerTest.java | 13 +++++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/java/java-psi-impl/src/com/intellij/core/CoreJavaFileManager.java b/java/java-psi-impl/src/com/intellij/core/CoreJavaFileManager.java index 4fb3ab0e8ebe..aa9c9801cc21 100644 --- a/java/java-psi-impl/src/com/intellij/core/CoreJavaFileManager.java +++ b/java/java-psi-impl/src/com/intellij/core/CoreJavaFileManager.java @@ -144,15 +144,22 @@ public class CoreJavaFileManager implements JavaFileManager { @Nullable private static PsiClass findClassInPsiFile(@NotNull String classNameWithInnerClassesDotSeparated, @NotNull PsiClassOwner file) { - final PsiClass[] classes = file.getClasses(); - if (classes.length != 1) { - return null; + for (PsiClass topLevelClass : file.getClasses()) { + PsiClass candidate = findClassByTopLevelClass(classNameWithInnerClassesDotSeparated, topLevelClass); + if (candidate != null) { + return candidate; + } } - PsiClass curClass = classes[0]; + return null; + } + + @Nullable + private static PsiClass findClassByTopLevelClass(@NotNull String classNameWithInnerClassesDotSeparated, @NotNull PsiClass topLevelClass) { Iterator segments = StringUtil.split(classNameWithInnerClassesDotSeparated, ".").iterator(); - if (!segments.hasNext() || !segments.next().equals(curClass.getName())) { + if (!segments.hasNext() || !segments.next().equals(topLevelClass.getName())) { return null; } + PsiClass curClass = topLevelClass; while (segments.hasNext()) { String innerClassName = segments.next(); PsiClass innerClass = curClass.findInnerClassByName(innerClassName, false); diff --git a/java/java-tests/testSrc/com/intellij/psi/CoreJavaFileManagerTest.java b/java/java-tests/testSrc/com/intellij/psi/CoreJavaFileManagerTest.java index c44eed5977e1..b7ebf4a1e10b 100644 --- a/java/java-tests/testSrc/com/intellij/psi/CoreJavaFileManagerTest.java +++ b/java/java-tests/testSrc/com/intellij/psi/CoreJavaFileManagerTest.java @@ -161,6 +161,19 @@ public class CoreJavaFileManagerTest extends PsiTestCase { assertCannotFind(fileWithEmptyName, ".foo"); } + public void testSeveralClassesInOneFile() throws Exception { + CoreJavaFileManager manager = configureManager("package foo;\n\n" + + "public class One {}\n" + + "class Two {}\n" + + "class Three {}", "One"); + + assertCanFind(manager, "foo.One"); + + //NOTE: this is unsupported + assertCannotFind(manager, "foo.Two"); + assertCannotFind(manager, "foo.Three"); + } + @NotNull private CoreJavaFileManager configureManager(@Language("JAVA") @NotNull String text, @NotNull String className) throws Exception { VirtualFile root = PsiTestUtil.createTestProjectStructure(myProject, myModule, myFilesToDelete); From d5bd063a5a0898376faa8c96dac7948dd3e2c40c Mon Sep 17 00:00:00 2001 From: "Pavel V. Talanov" Date: Tue, 13 Jan 2015 19:03:35 +0300 Subject: [PATCH 05/12] CoreJavaFileManager: check scope when searching for classes --- .../src/com/intellij/core/CoreJavaFileManager.java | 12 +++++++++--- .../com/intellij/psi/CoreJavaFileManagerTest.java | 7 +++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/java/java-psi-impl/src/com/intellij/core/CoreJavaFileManager.java b/java/java-psi-impl/src/com/intellij/core/CoreJavaFileManager.java index aa9c9801cc21..ffd04e83b4ae 100644 --- a/java/java-psi-impl/src/com/intellij/core/CoreJavaFileManager.java +++ b/java/java-psi-impl/src/com/intellij/core/CoreJavaFileManager.java @@ -84,7 +84,7 @@ public class CoreJavaFileManager implements JavaFileManager { @Override public PsiClass findClass(@NotNull String qName, @NotNull GlobalSearchScope scope) { for (VirtualFile root : roots()) { - final PsiClass psiClass = findClassInClasspathRoot(qName, root, myPsiManager); + final PsiClass psiClass = findClassInClasspathRoot(qName, root, myPsiManager, scope); if (psiClass != null) { return psiClass; } @@ -93,7 +93,10 @@ public class CoreJavaFileManager implements JavaFileManager { } @Nullable - public static PsiClass findClassInClasspathRoot(String qName, VirtualFile root, PsiManager psiManager) { + public static PsiClass findClassInClasspathRoot(@NotNull String qName, + @NotNull VirtualFile root, + @NotNull PsiManager psiManager, + @NotNull GlobalSearchScope scope) { String pathRest = qName; VirtualFile cur = root; @@ -122,6 +125,9 @@ public class CoreJavaFileManager implements JavaFileManager { LOG.error("Invalid child of valid parent: " + vFile.getPath() + "; " + root.isValid() + " path=" + root.getPath()); return null; } + if (!scope.contains(vFile)) { + return null; + } final PsiFile file = psiManager.findFile(vFile); if (!(file instanceof PsiClassOwner)) { @@ -176,7 +182,7 @@ public class CoreJavaFileManager implements JavaFileManager { public PsiClass[] findClasses(@NotNull String qName, @NotNull GlobalSearchScope scope) { List result = new ArrayList(); for (VirtualFile file : roots()) { - final PsiClass psiClass = findClassInClasspathRoot(qName, file, myPsiManager); + final PsiClass psiClass = findClassInClasspathRoot(qName, file, myPsiManager, scope); if (psiClass != null) { result.add(psiClass); } diff --git a/java/java-tests/testSrc/com/intellij/psi/CoreJavaFileManagerTest.java b/java/java-tests/testSrc/com/intellij/psi/CoreJavaFileManagerTest.java index b7ebf4a1e10b..6c61515bfee1 100644 --- a/java/java-tests/testSrc/com/intellij/psi/CoreJavaFileManagerTest.java +++ b/java/java-tests/testSrc/com/intellij/psi/CoreJavaFileManagerTest.java @@ -174,6 +174,13 @@ public class CoreJavaFileManagerTest extends PsiTestCase { assertCannotFind(manager, "foo.Three"); } + public void testScopeCheck() throws Exception { + CoreJavaFileManager manager = configureManager("package foo;\n\n" + "public class Test {}\n", "Test"); + + assertNotNull("Should find class in all scope", manager.findClass("foo.Test", GlobalSearchScope.allScope(getProject()))); + assertNull("Should not find class in empty scope", manager.findClass("foo.Test", GlobalSearchScope.EMPTY_SCOPE)); + } + @NotNull private CoreJavaFileManager configureManager(@Language("JAVA") @NotNull String text, @NotNull String className) throws Exception { VirtualFile root = PsiTestUtil.createTestProjectStructure(myProject, myModule, myFilesToDelete); From f9bbc71f84a6ef7ad9aa5605893497b05b76ddfa Mon Sep 17 00:00:00 2001 From: "Pavel V. Talanov" Date: Tue, 13 Jan 2015 20:51:54 +0300 Subject: [PATCH 06/12] CoreJavaFileManager: avoid constructing unneeded list --- .../src/com/intellij/core/CoreJavaFileManager.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/java/java-psi-impl/src/com/intellij/core/CoreJavaFileManager.java b/java/java-psi-impl/src/com/intellij/core/CoreJavaFileManager.java index ffd04e83b4ae..a7d671870934 100644 --- a/java/java-psi-impl/src/com/intellij/core/CoreJavaFileManager.java +++ b/java/java-psi-impl/src/com/intellij/core/CoreJavaFileManager.java @@ -160,8 +160,12 @@ public class CoreJavaFileManager implements JavaFileManager { } @Nullable - private static PsiClass findClassByTopLevelClass(@NotNull String classNameWithInnerClassesDotSeparated, @NotNull PsiClass topLevelClass) { - Iterator segments = StringUtil.split(classNameWithInnerClassesDotSeparated, ".").iterator(); + private static PsiClass findClassByTopLevelClass(@NotNull String className, @NotNull PsiClass topLevelClass) { + if (className.indexOf('.') < 0) { + return className.equals(topLevelClass.getName()) ? topLevelClass : null; + } + + Iterator segments = StringUtil.split(className, ".").iterator(); if (!segments.hasNext() || !segments.next().equals(topLevelClass.getName())) { return null; } From 7e7608c8009500729329a3be3c56d2bca75c5e01 Mon Sep 17 00:00:00 2001 From: "Egor.Ushakov" Date: Wed, 14 Jan 2015 14:28:55 +0300 Subject: [PATCH 07/12] IDEA-131754 Catch and finally blocks have no line information --- .../modules/code/DeadCodeHelper.java | 3 ++- .../engine/testData/results/TestClassLoop.dec | 10 ++++++++-- .../TestClassSimpleBytecodeMapping.dec | 11 +++++++++-- .../engine/testData/results/TestClassVar.dec | 5 ++++- .../results/TestSynchronizedMapping.dec | 1 + .../testData/results/TestTryCatchFinally.dec | 19 ++++++++++++++++--- 6 files changed, 40 insertions(+), 9 deletions(-) diff --git a/plugins/java-decompiler/engine/src/org/jetbrains/java/decompiler/modules/code/DeadCodeHelper.java b/plugins/java-decompiler/engine/src/org/jetbrains/java/decompiler/modules/code/DeadCodeHelper.java index c7dd8ca1c173..233011df296a 100644 --- a/plugins/java-decompiler/engine/src/org/jetbrains/java/decompiler/modules/code/DeadCodeHelper.java +++ b/plugins/java-decompiler/engine/src/org/jetbrains/java/decompiler/modules/code/DeadCodeHelper.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2014 JetBrains s.r.o. + * Copyright 2000-2015 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. @@ -416,6 +416,7 @@ public class DeadCodeHelper { if (sameRanges) { seq.addSequence(next.getSeq()); + block.getInstrOldOffsets().addAll(next.getInstrOldOffsets()); next.getSeq().clear(); removeEmptyBlock(graph, next, true); diff --git a/plugins/java-decompiler/engine/testData/results/TestClassLoop.dec b/plugins/java-decompiler/engine/testData/results/TestClassLoop.dec index 5384a981e0f3..f3bf0e07304d 100644 --- a/plugins/java-decompiler/engine/testData/results/TestClassLoop.dec +++ b/plugins/java-decompiler/engine/testData/results/TestClassLoop.dec @@ -16,7 +16,7 @@ public class TestClassLoop { return; } } finally { - System.out.println("1"); + System.out.println("1");// 38 } } } @@ -30,7 +30,7 @@ public class TestClassLoop { System.out.println("1");// 49 break; } finally { - if(var0) { + if(var0) {// 52 System.out.println("3");// 53 continue; } @@ -54,6 +54,9 @@ class 'pkg/TestClassLoop' { 4 10 d 10 f 14 + 26 18 + 27 18 + 2a 18 } method 'testFinallyContinue ()V' { @@ -64,6 +67,7 @@ class 'pkg/TestClassLoop' { e 29 11 29 13 29 + 26 32 2a 33 2d 33 2f 33 @@ -74,6 +78,8 @@ Lines mapping: 23 <-> 6 29 <-> 11 33 <-> 15 +38 <-> 19 45 <-> 25 49 <-> 30 +52 <-> 33 53 <-> 34 diff --git a/plugins/java-decompiler/engine/testData/results/TestClassSimpleBytecodeMapping.dec b/plugins/java-decompiler/engine/testData/results/TestClassSimpleBytecodeMapping.dec index 291151311a18..b6b6bba690cb 100644 --- a/plugins/java-decompiler/engine/testData/results/TestClassSimpleBytecodeMapping.dec +++ b/plugins/java-decompiler/engine/testData/results/TestClassSimpleBytecodeMapping.dec @@ -22,9 +22,9 @@ public class TestClassSimpleBytecodeMapping { try { Integer.parseInt(var1);// 34 } catch (Exception var6) { - System.out.println(var6); + System.out.println(var6);// 36 } finally { - System.out.println("Finally"); + System.out.println("Finally");// 38 } } @@ -80,6 +80,11 @@ class 'pkg/TestClassSimpleBytecodeMapping' { method 'test2 (Ljava/lang/String;)V' { 1 22 + 11 24 + 15 24 + 23 26 + 24 26 + 27 26 } method 'run (Ljava/lang/Runnable;)V' { @@ -114,6 +119,8 @@ Lines mapping: 27 <-> 16 28 <-> 17 34 <-> 23 +36 <-> 25 +38 <-> 27 44 <-> 44 49 <-> 33 54 <-> 38 diff --git a/plugins/java-decompiler/engine/testData/results/TestClassVar.dec b/plugins/java-decompiler/engine/testData/results/TestClassVar.dec index 14d2e070393d..cedd9c0d80a0 100644 --- a/plugins/java-decompiler/engine/testData/results/TestClassVar.dec +++ b/plugins/java-decompiler/engine/testData/results/TestClassVar.dec @@ -9,7 +9,7 @@ public class TestClassVar { try { System.out.println();// 29 } finally { - if(this.field_boolean) { + if(this.field_boolean) {// 32 System.out.println();// 33 } @@ -46,6 +46,8 @@ class 'pkg/TestClassVar' { 3 7 8 9 b 9 + 1f 11 + 20 11 26 12 29 12 } @@ -73,6 +75,7 @@ class 'pkg/TestClassVar' { Lines mapping: 26 <-> 8 29 <-> 10 +32 <-> 12 33 <-> 13 40 <-> 22 45 <-> 26 diff --git a/plugins/java-decompiler/engine/testData/results/TestSynchronizedMapping.dec b/plugins/java-decompiler/engine/testData/results/TestSynchronizedMapping.dec index ca38d8c0e732..6ba9f3352815 100644 --- a/plugins/java-decompiler/engine/testData/results/TestSynchronizedMapping.dec +++ b/plugins/java-decompiler/engine/testData/results/TestSynchronizedMapping.dec @@ -16,6 +16,7 @@ class 'pkg/TestSynchronizedMapping' { method 'test (I)I' { 3 4 5 5 + a 5 } method 'test2 (Ljava/lang/String;)V' { diff --git a/plugins/java-decompiler/engine/testData/results/TestTryCatchFinally.dec b/plugins/java-decompiler/engine/testData/results/TestTryCatchFinally.dec index 673238ec14fb..2dc71a58633f 100644 --- a/plugins/java-decompiler/engine/testData/results/TestTryCatchFinally.dec +++ b/plugins/java-decompiler/engine/testData/results/TestTryCatchFinally.dec @@ -11,7 +11,7 @@ public class TestTryCatchFinally { ; } } finally { - System.out.println("finally"); + System.out.println("finally");// 34 } } @@ -31,9 +31,9 @@ public class TestTryCatchFinally { int var2 = Integer.parseInt(var1);// 51 return var2; } catch (Exception var6) { - System.out.println("Error" + var6); + System.out.println("Error" + var6);// 53 } finally { - System.out.println("Finally"); + System.out.println("Finally");// 55 } return -1; @@ -48,6 +48,9 @@ class 'pkg/TestTryCatchFinally' { 14 8 17 8 19 8 + 2b 13 + 2d 13 + 30 13 } method 'foo (I)I' { @@ -63,15 +66,25 @@ class 'pkg/TestTryCatchFinally' { method 'test (Ljava/lang/String;)I' { 1 30 4 30 + 10 33 + 1a 33 + 23 33 + 26 33 + 34 35 + 35 35 + 38 35 } } Lines mapping: 24 <-> 6 27 <-> 9 +34 <-> 14 39 <-> 20 40 <-> 21 41 <-> 22 42 <-> 23 45 <-> 25 51 <-> 31 +53 <-> 34 +55 <-> 36 From 34441726d64cd97f4018ab53fff3f911485a931d Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Wed, 14 Jan 2015 11:26:25 +0100 Subject: [PATCH 08/12] IDEA-135185 (Exception in "Cyclic class dependency" inspection) --- .../CyclicClassDependencyInspection.java | 15 ++++----------- .../cyclic_class_dependency/expected.xml | 7 ------- .../cyclic_class_dependency/src/Cyclic.java | 15 +++++++++++++++ 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/dependency/CyclicClassDependencyInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/dependency/CyclicClassDependencyInspection.java index cb512f97ecbd..b501653d6dc1 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/dependency/CyclicClassDependencyInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/dependency/CyclicClassDependencyInspection.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2013 Dave Griffith, Bas Leijdekkers + * Copyright 2006-2015 Dave Griffith, Bas Leijdekkers * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -52,7 +52,7 @@ public class CyclicClassDependencyInspection extends BaseGlobalInspection { } final RefClass refClass = (RefClass)refEntity; final PsiClass aClass = refClass.getElement(); - if (aClass == null || aClass.getContainingClass() != null) { + if (aClass == null || aClass.getContainingClass() != null || aClass instanceof PsiAnonymousClass) { return null; } final Set dependencies = DependencyUtils.calculateTransitiveDependenciesForClass(refClass); @@ -79,15 +79,8 @@ public class CyclicClassDependencyInspection extends BaseGlobalInspection { errorString = InspectionGadgetsBundle.message("cyclic.class.dependency.problem.descriptor", refEntity.getName(), Integer.valueOf(numMutualDependents)); } - final PsiElement anchor; - if (aClass instanceof PsiAnonymousClass) { - final PsiAnonymousClass anonymousClass = (PsiAnonymousClass)aClass; - anchor = anonymousClass.getBaseClassReference(); - } - else { - anchor = aClass.getNameIdentifier(); - if (anchor == null) return null; - } + final PsiElement anchor = aClass.getNameIdentifier(); + if (anchor == null) return null; return new CommonProblemDescriptor[]{ inspectionManager.createProblemDescriptor(anchor, errorString, (LocalQuickFix)null, ProblemHighlightType.GENERIC_ERROR_OR_WARNING, false) diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/cyclic_class_dependency/expected.xml b/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/cyclic_class_dependency/expected.xml index 4e19f4850c1b..d75c4d2e2d95 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/cyclic_class_dependency/expected.xml +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/cyclic_class_dependency/expected.xml @@ -1,12 +1,5 @@ - - Cyclic.java - 9 - Cyclic class dependency - Class 'anonymous (java.lang.Object)' is cyclically dependent on 3 other classes - - Cyclic.java 17 diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/cyclic_class_dependency/src/Cyclic.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/cyclic_class_dependency/src/Cyclic.java index e0f9cf48061a..e8f750c99fa3 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/cyclic_class_dependency/src/Cyclic.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/cyclic_class_dependency/src/Cyclic.java @@ -27,3 +27,18 @@ interface FiveOClock { } interface Coffee extends FiveOClock {} +enum MyEnum { + ONE { + public int value() { + return 0; + } + }, + + TWO { + public int value() { + return ONE.value(); + } + }; + + abstract int value(); +} From f8a49b10a3a90b60520aaeac830c2a8bf15f6bfe Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Wed, 14 Jan 2015 11:35:44 +0100 Subject: [PATCH 09/12] IG: fix option so it is useful --- .../siyeh/InspectionGadgetsBundle.properties | 3 ++- ...oneDeclaresCloneNotSupportedInspection.java | 18 +++++++++--------- .../CloneDeclaresCloneNotSupported.html | 4 ++-- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties index 459950f67bf8..547635b92ad1 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties @@ -2135,7 +2135,8 @@ utility.class.code.can.be.enum.quickfix=Convert to 'enum' non.public.clone.display.name='clone()' method not 'public' non.public.clone.problem.descriptor=#ref() method not 'public' #loc only.warn.on.public.clone.methods=Only warn on 'public' clone methods +only.warn.on.protected.clone.methods=Only warn on 'protected' clone methods clone.returns.class.type.display.name='clone()' should have return type equal to the class it contains clone.returns.class.type.problem.descriptor=''clone()'' should have return type ''{0}'' #loc -clone.returns.class.type.quickfix=Change return type to '{0}' +clone.returns.class.type.quickfix=Change return type to ''{0}'' clone.returns.class.type.family.quickfix=Change return type to class type diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/cloneable/CloneDeclaresCloneNotSupportedInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/cloneable/CloneDeclaresCloneNotSupportedInspection.java index 3f605269bff4..b91bec416a28 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/cloneable/CloneDeclaresCloneNotSupportedInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/cloneable/CloneDeclaresCloneNotSupportedInspection.java @@ -39,7 +39,7 @@ import javax.swing.*; public class CloneDeclaresCloneNotSupportedInspection extends BaseInspection { - private boolean onlyWarnOnPublicClone = true; + private boolean onlyWarnOnProtectedClone = true; @Override @NotNull @@ -67,16 +67,16 @@ public class CloneDeclaresCloneNotSupportedInspection extends BaseInspection { @Nullable @Override public JComponent createOptionsPanel() { - return new SingleCheckboxOptionsPanel(InspectionGadgetsBundle.message("only.warn.on.public.clone.methods"), - this, "onlyWarnOnPublicClone"); + return new SingleCheckboxOptionsPanel(InspectionGadgetsBundle.message("only.warn.on.protected.clone.methods"), + this, "onlyWarnOnProtectedClone"); } @Override public void readSettings(@NotNull Element node) throws InvalidDataException { super.readSettings(node); for (Element option : node.getChildren("option")) { - if ("onlyWarnOnPublicClone".equals(option.getAttributeValue("name"))) { - onlyWarnOnPublicClone = Boolean.parseBoolean(option.getAttributeValue("value")); + if ("onlyWarnOnProtectedClone".equals(option.getAttributeValue("name"))) { + onlyWarnOnProtectedClone = Boolean.parseBoolean(option.getAttributeValue("value")); } } } @@ -84,9 +84,9 @@ public class CloneDeclaresCloneNotSupportedInspection extends BaseInspection { @Override public void writeSettings(@NotNull Element node) throws WriteExternalException { super.writeSettings(node); - if (!onlyWarnOnPublicClone) { - node.addContent(new Element("option").setAttribute("name", "onlyWarnOnPublicClone") - .setAttribute("value", String.valueOf(onlyWarnOnPublicClone))); + if (!onlyWarnOnProtectedClone) { + node.addContent(new Element("option").setAttribute("name", "onlyWarnOnProtectedClone") + .setAttribute("value", String.valueOf(onlyWarnOnProtectedClone))); } } @@ -131,7 +131,7 @@ public class CloneDeclaresCloneNotSupportedInspection extends BaseInspection { if (method.hasModifierProperty(PsiModifier.FINAL)) { return; } - if (onlyWarnOnPublicClone && !method.hasModifierProperty(PsiModifier.PUBLIC)) { + if (onlyWarnOnProtectedClone && method.hasModifierProperty(PsiModifier.PUBLIC)) { return; } final PsiClass containingClass = method.getContainingClass(); diff --git a/plugins/InspectionGadgets/src/inspectionDescriptions/CloneDeclaresCloneNotSupported.html b/plugins/InspectionGadgets/src/inspectionDescriptions/CloneDeclaresCloneNotSupported.html index e50bfac4f8df..b1f175d56f7e 100644 --- a/plugins/InspectionGadgets/src/inspectionDescriptions/CloneDeclaresCloneNotSupported.html +++ b/plugins/InspectionGadgets/src/inspectionDescriptions/CloneDeclaresCloneNotSupported.html @@ -8,9 +8,9 @@ prohibit cloning will not be able to do so in the standard way. This inspection or clone() methods on final classes.

-Use the checkbox below to indicate if this inspection should only warn on public methods. +Use the checkbox below to indicate if this inspection should only warn on protected methods. In Effective Java, Second Edition (but not in the first edition) it is recommended to omit the CloneNotSupportedException -declaration, because methods that don't throw checked exceptions are easier to use. +declaration on public methods, because methods that don't throw checked exceptions are easier to use.

From 852b48aaaf946fc3ff23490d1c5c320ed5cf4a4c Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Wed, 14 Jan 2015 11:44:26 +0100 Subject: [PATCH 10/12] make IG test light and test new option --- .../CloneDeclaresCloneNonSupportedException.java | 12 +++++++++++- .../expected.xml | 9 --------- ...eDeclaresCloneNotSupportedInspectionTest.java | 16 ++++++++++++---- 3 files changed, 23 insertions(+), 14 deletions(-) delete mode 100644 plugins/InspectionGadgets/test/com/siyeh/igtest/cloneable/clone_declares_clone_not_supported/expected.xml diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/cloneable/clone_declares_clone_not_supported/CloneDeclaresCloneNonSupportedException.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/cloneable/clone_declares_clone_not_supported/CloneDeclaresCloneNonSupportedException.java index 461fd61ca5d7..4c71a4ad77ef 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/cloneable/clone_declares_clone_not_supported/CloneDeclaresCloneNonSupportedException.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/cloneable/clone_declares_clone_not_supported/CloneDeclaresCloneNonSupportedException.java @@ -8,7 +8,7 @@ public class CloneDeclaresCloneNonSupportedException implements Cloneable } - public Object clone() + protected Object clone() { try { @@ -34,3 +34,13 @@ class Child extends CloneDeclaresCloneNonSupportedException { return super.clone(); } } +class NoWarnOnPublic implements Cloneable { + + public Object clone() { + try { + return super.clone(); + } catch (CloneNotSupportedException e) { + throw new AssertionError(e); + } + } +} diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/cloneable/clone_declares_clone_not_supported/expected.xml b/plugins/InspectionGadgets/test/com/siyeh/igtest/cloneable/clone_declares_clone_not_supported/expected.xml deleted file mode 100644 index 017ae12ebb44..000000000000 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/cloneable/clone_declares_clone_not_supported/expected.xml +++ /dev/null @@ -1,9 +0,0 @@ - - - - CloneDeclaresCloneNonSupportedException.java - 11 - 'clone()' does not declare 'CloneNotSupportedException' - <code>clone()</code> does not declare 'CloneNotSupportedException' - - \ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/cloneable/CloneDeclaresCloneNotSupportedInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/cloneable/CloneDeclaresCloneNotSupportedInspectionTest.java index 5b88db3d2659..3f3a0e373801 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/cloneable/CloneDeclaresCloneNotSupportedInspectionTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/cloneable/CloneDeclaresCloneNotSupportedInspectionTest.java @@ -1,10 +1,18 @@ package com.siyeh.ig.cloneable; -import com.siyeh.ig.IGInspectionTestCase; +import com.intellij.codeInspection.InspectionProfileEntry; +import com.siyeh.ig.LightInspectionTestCase; +import org.jetbrains.annotations.Nullable; -public class CloneDeclaresCloneNotSupportedInspectionTest extends IGInspectionTestCase { +public class CloneDeclaresCloneNotSupportedInspectionTest extends LightInspectionTestCase { - public void test() throws Exception { - doTest("com/siyeh/igtest/cloneable/clone_declares_clone_not_supported", new CloneDeclaresCloneNotSupportedInspection()); + public void testCloneDeclaresCloneNonSupportedException() { + doTest(); + } + + @Nullable + @Override + protected InspectionProfileEntry getInspection() { + return new CloneDeclaresCloneNotSupportedInspection(); } } \ No newline at end of file From 3f2212a179f4ae7a9166def4323866b59aef681c Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Wed, 14 Jan 2015 12:35:09 +0100 Subject: [PATCH 11/12] IDEA-135189 (False positive of "Type may be weakened") --- .../TypeMayBeWeakenedInspection.java | 5 ++-- .../siyeh/ig/psiutils/WeakestTypeFinder.java | 14 +++++++---- .../abstraction/weaken_type/Lambda.java | 23 +++++++++++++++++++ .../TypeMayBeWeakenedInspectionTest.java | 6 +++++ 4 files changed, 42 insertions(+), 6 deletions(-) create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igtest/abstraction/weaken_type/Lambda.java diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/abstraction/TypeMayBeWeakenedInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/abstraction/TypeMayBeWeakenedInspection.java index 2787f34e4dcf..ec2778f79c5b 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/abstraction/TypeMayBeWeakenedInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/abstraction/TypeMayBeWeakenedInspection.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2014 Bas Leijdekkers + * Copyright 2006-2015 Bas Leijdekkers * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,6 +18,7 @@ package com.siyeh.ig.abstraction; import com.intellij.codeInsight.daemon.impl.analysis.JavaHighlightUtil; import com.intellij.codeInspection.ProblemDescriptor; import com.intellij.codeInspection.ui.MultipleCheckboxOptionsPanel; +import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.project.Project; import com.intellij.psi.*; import com.intellij.psi.codeStyle.JavaCodeStyleManager; @@ -294,7 +295,7 @@ public class TypeMayBeWeakenedInspection extends BaseInspection { @Override public void visitMethod(PsiMethod method) { super.visitMethod(method); - if (isOnTheFly() && !method.hasModifierProperty(PsiModifier.PRIVATE)) { + if (isOnTheFly() && !method.hasModifierProperty(PsiModifier.PRIVATE) && !ApplicationManager.getApplication().isUnitTestMode()) { // checking methods with greater visibility is too expensive. // for error checking in the editor return; diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/WeakestTypeFinder.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/WeakestTypeFinder.java index 835d841e0de8..23851eed4952 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/WeakestTypeFinder.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/WeakestTypeFinder.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2014 Bas Leijdekkers + * Copyright 2008-2015 Bas Leijdekkers * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -155,11 +155,17 @@ public class WeakestTypeFinder { checkClass(javaLangIterableClass, weakestTypeClasses); } else if (referenceParent instanceof PsiReturnStatement) { - final PsiMethod containingMethod = PsiTreeUtil.getParentOfType(referenceParent, PsiMethod.class); - if (containingMethod == null) { + final PsiElement owner = PsiTreeUtil.getParentOfType(referenceParent, PsiMethod.class, PsiLambdaExpression.class); + final PsiType type; + if (owner instanceof PsiMethod) { + type = ((PsiMethod)owner).getReturnType(); + } + else if (owner instanceof PsiLambdaExpression) { + type = LambdaUtil.getFunctionalInterfaceReturnType((PsiLambdaExpression)owner); + } + else { return Collections.emptyList(); } - final PsiType type = containingMethod.getReturnType(); if (!checkType(type, weakestTypeClasses)) { return Collections.emptyList(); } diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/abstraction/weaken_type/Lambda.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/abstraction/weaken_type/Lambda.java new file mode 100644 index 000000000000..01cdacd79161 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/abstraction/weaken_type/Lambda.java @@ -0,0 +1,23 @@ +package com.siyeh.igtest.abstraction.weaken_type; + +import java.util.function.Supplier; + +public interface Lambda { + + int count(); + + static Lambda newWeaken(int value) { + return new Lambda() { + @Override + public int count() { + return value; + } + }; + } + + static Supplier newSupplier() { + return () -> { + return Lambda.newWeaken(0); + }; + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/abstraction/TypeMayBeWeakenedInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/abstraction/TypeMayBeWeakenedInspectionTest.java index 52e5e8cc3db2..20e587914e40 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/abstraction/TypeMayBeWeakenedInspectionTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/abstraction/TypeMayBeWeakenedInspectionTest.java @@ -9,6 +9,7 @@ public class TypeMayBeWeakenedInspectionTest extends LightInspectionTestCase { public void testTypeMayBeWeakened() { doTest(); } public void testNumberAdderDemo() { doTest(); } public void testAutoClosableTest() { doTest(); } + public void testLambda() { doTest(); } @Override protected String[] getEnvironmentClasses() { @@ -33,6 +34,11 @@ public class TypeMayBeWeakenedInspectionTest extends LightInspectionTestCase { "@FunctionalInterface " + "public interface Function {" + " R apply(T t);" + + "}", + "package java.util.function;\n" + + "@FunctionalInterface\n" + + "public interface Supplier {\n" + + " T get();\n" + "}" }; } From 1854a24555bebb3142bc137a7f49366aa7ae9bca Mon Sep 17 00:00:00 2001 From: Anton Makeev Date: Wed, 14 Jan 2015 12:42:16 +0100 Subject: [PATCH 12/12] Platform: help topic for 'Non-project file edit' dialog (IDEA-121829) + clarified options' test IDEA-133451 Default action for return key unlocks framework headers --- .../fileEditor/impl/NonProjectFileWritingAccessDialog.form | 4 ++-- .../fileEditor/impl/NonProjectFileWritingAccessDialog.java | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/NonProjectFileWritingAccessDialog.form b/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/NonProjectFileWritingAccessDialog.form index 33a20bc8b164..01173794cbeb 100644 --- a/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/NonProjectFileWritingAccessDialog.form +++ b/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/NonProjectFileWritingAccessDialog.form @@ -22,7 +22,7 @@ - + @@ -30,7 +30,7 @@ - + diff --git a/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/NonProjectFileWritingAccessDialog.java b/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/NonProjectFileWritingAccessDialog.java index f85a0bf52ab1..397ff73da184 100644 --- a/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/NonProjectFileWritingAccessDialog.java +++ b/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/NonProjectFileWritingAccessDialog.java @@ -38,9 +38,13 @@ public class NonProjectFileWritingAccessDialog extends DialogWrapper { myFileList.setCellRenderer(new FileListRenderer()); myFileList.setModel(new CollectionListModel(nonProjectFiles)); + getOKAction().putValue(DEFAULT_ACTION, null); + getCancelAction().putValue(DEFAULT_ACTION, true); + init(); } + @Nullable @Override protected JComponent createCenterPanel() { @@ -54,6 +58,6 @@ public class NonProjectFileWritingAccessDialog extends DialogWrapper { } protected String getHelpId() { - return "readOnlyHandler.nonProjectFilesDialog"; + return "Non-Project_Files_Access_Dialog"; } }