From 2d0aa2ae3af2525e298f8221bba45b1ceac4713b Mon Sep 17 00:00:00 2001 From: Dmitry Batkovich Date: Thu, 29 May 2014 18:03:30 +0400 Subject: [PATCH] BlockMarkerCommentsInspection changes after review --- .../BlockMarkerCommentsInspection.java | 126 ++++++++---------- .../inspection/blockMarkerComments/Class.java | 25 ++++ .../{if/src/Foo.java => If.java} | 4 +- .../inspection/blockMarkerComments/Loop.java | 31 +++++ .../{method/src/Foo.java => Method.java} | 2 +- ...lockMarker.java => RemoveBlockMarker.java} | 0 ...fter.java => RemoveBlockMarker_after.java} | 0 .../blockMarkerComments/TryCatch.java | 45 +++++++ .../blockMarkerComments/class/expected.xml | 22 --- .../blockMarkerComments/class/src/Foo.java | 18 --- .../blockMarkerComments/if/expected.xml | 15 --- .../blockMarkerComments/loop/expected.xml | 15 --- .../blockMarkerComments/loop/src/Foo.java | 25 ---- .../blockMarkerComments/method/expected.xml | 9 -- .../blockMarkerComments/tryCatch/expected.xml | 27 ---- .../blockMarkerComments/tryCatch/src/Foo.java | 44 ------ .../BlockMarkerCommentsTest.java | 23 +++- resources/src/META-INF/IdeaPlugin.xml | 2 +- 18 files changed, 178 insertions(+), 255 deletions(-) create mode 100644 java/java-tests/testData/inspection/blockMarkerComments/Class.java rename java/java-tests/testData/inspection/blockMarkerComments/{if/src/Foo.java => If.java} (70%) create mode 100644 java/java-tests/testData/inspection/blockMarkerComments/Loop.java rename java/java-tests/testData/inspection/blockMarkerComments/{method/src/Foo.java => Method.java} (52%) rename java/java-tests/testData/inspection/blockMarkerComments/{removeBlockMarker.java => RemoveBlockMarker.java} (100%) rename java/java-tests/testData/inspection/blockMarkerComments/{removeBlockMarker_after.java => RemoveBlockMarker_after.java} (100%) create mode 100644 java/java-tests/testData/inspection/blockMarkerComments/TryCatch.java delete mode 100644 java/java-tests/testData/inspection/blockMarkerComments/class/expected.xml delete mode 100644 java/java-tests/testData/inspection/blockMarkerComments/class/src/Foo.java delete mode 100644 java/java-tests/testData/inspection/blockMarkerComments/if/expected.xml delete mode 100644 java/java-tests/testData/inspection/blockMarkerComments/loop/expected.xml delete mode 100644 java/java-tests/testData/inspection/blockMarkerComments/loop/src/Foo.java delete mode 100644 java/java-tests/testData/inspection/blockMarkerComments/method/expected.xml delete mode 100644 java/java-tests/testData/inspection/blockMarkerComments/tryCatch/expected.xml delete mode 100644 java/java-tests/testData/inspection/blockMarkerComments/tryCatch/src/Foo.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/BlockMarkerCommentsInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/BlockMarkerCommentsInspection.java index ecfd3de12521..f2d8f77267a7 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/BlockMarkerCommentsInspection.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/BlockMarkerCommentsInspection.java @@ -15,17 +15,58 @@ */ package com.intellij.codeInspection; +import com.intellij.lang.Commenter; +import com.intellij.lang.LanguageCommenters; import com.intellij.openapi.project.Project; +import com.intellij.openapi.util.text.StringUtil; +import com.intellij.patterns.ElementPattern; +import com.intellij.patterns.PatternCondition; +import com.intellij.patterns.PsiJavaElementPattern; import com.intellij.psi.*; import com.intellij.psi.tree.IElementType; -import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.util.ProcessingContext; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; +import static com.intellij.patterns.PsiJavaPatterns.*; + /** * @author Dmitry Batkovich */ public class BlockMarkerCommentsInspection extends BaseJavaBatchLocalInspectionTool { + private static final PsiJavaElementPattern ANONYMOUS_CLASS_MARKER_PATTERN = psiElement(). + withParent(psiElement(PsiDeclarationStatement.class, PsiExpressionStatement.class)) + .afterSiblingSkipping(or(psiElement(PsiWhiteSpace.class), psiElement(PsiJavaToken.class).with(new PatternCondition(null) { + @Override + public boolean accepts(@NotNull final PsiJavaToken psiJavaToken, final ProcessingContext context) { + return psiJavaToken.getTokenType().equals(JavaTokenType.SEMICOLON); + } + })), + psiElement(PsiLocalVariable.class, PsiAssignmentExpression.class) + .withChild(psiElement(PsiNewExpression.class).withChild(psiElement(PsiAnonymousClass.class)))); + private static final PsiJavaElementPattern CLASS_MARKER_PATTERN = psiElement(). + withParent(PsiClass.class). + afterSiblingSkipping(psiElement(PsiWhiteSpace.class), psiElement(PsiJavaToken.class).with(new PatternCondition(null) { + @Override + public boolean accepts(@NotNull final PsiJavaToken token, final ProcessingContext context) { + return JavaTokenType.RBRACE.equals(token.getTokenType()); + } + })); + private static final PsiJavaElementPattern TRY_CATCH_MARKER_PATTERN = psiElement(). + withParent(PsiTryStatement.class). + afterSiblingSkipping(psiElement(PsiWhiteSpace.class), psiElement(PsiCodeBlock.class, PsiCatchSection.class)); + private static final PsiJavaElementPattern LOOP_OR_IF_MARKER = + psiElement().afterSiblingSkipping(psiElement(PsiWhiteSpace.class), psiElement(PsiCodeBlock.class)). + withParent(psiElement(PsiBlockStatement.class).withParent(psiElement(PsiLoopStatement.class, PsiIfStatement.class))); + private static final PsiJavaElementPattern METHOD_MARKER_PATTERN = + psiElement().withParent(PsiMethod.class).afterSiblingSkipping(psiElement(PsiWhiteSpace.class), psiElement(PsiCodeBlock.class)); + + private static final ElementPattern MARKER_PATTERN = or(ANONYMOUS_CLASS_MARKER_PATTERN, + CLASS_MARKER_PATTERN, + TRY_CATCH_MARKER_PATTERN, + LOOP_OR_IF_MARKER, + METHOD_MARKER_PATTERN); + private static final String END_WORD = "end"; @NotNull @@ -33,24 +74,23 @@ public class BlockMarkerCommentsInspection extends BaseJavaBatchLocalInspectionT public PsiElementVisitor buildVisitor(@NotNull final ProblemsHolder holder, final boolean isOnTheFly) { return new PsiElementVisitor() { @Override - public void visitElement(final PsiElement element) { - if (!(element instanceof PsiComment)) { - return; - } - final IElementType tokenType = ((PsiComment)element).getTokenType(); + public void visitComment(final PsiComment element) { + final IElementType tokenType = element.getTokenType(); if (!(tokenType.equals(JavaTokenType.END_OF_LINE_COMMENT))) { return; } - final String commentText = element.getText().substring(2).trim().toLowerCase(); - if (!commentText.startsWith(END_WORD)) { + final Commenter commenter = LanguageCommenters.INSTANCE.forLanguage(element.getLanguage()); + String rawCommentText = element.getText(); + final String prefix = commenter.getLineCommentPrefix(); + if (prefix != null && rawCommentText.startsWith(prefix)) { + rawCommentText = rawCommentText.substring(prefix.length()); + } + final String commentText = rawCommentText.trim().toLowerCase(); + if (!commentText.startsWith(END_WORD) || StringUtil.split(commentText, " ").size() > 3) { return; } - if (isMethodBlockMarker(element) || - isLoopOrIfBlockMarker(element) || - isClassBlockMarker(element) || - isAnonymousClass(element) || - isTryCatchFinallyBlockMarker(element)) { - holder.registerProblem(element, "", new LocalQuickFix() { + if (MARKER_PATTERN.accepts(element)) { + holder.registerProblem(element, "Redundant block marker", new LocalQuickFix() { @NotNull @Override public String getName() { @@ -65,7 +105,7 @@ public class BlockMarkerCommentsInspection extends BaseJavaBatchLocalInspectionT @Override public void applyFix(@NotNull final Project project, @NotNull final ProblemDescriptor descriptor) { - element.delete(); + descriptor.getPsiElement().delete(); } }); } @@ -79,60 +119,4 @@ public class BlockMarkerCommentsInspection extends BaseJavaBatchLocalInspectionT public String getDisplayName() { return "Block marker comment"; } - - private static boolean isAnonymousClass(final PsiElement comment) { - final PsiElement parent = comment.getParent(); - if (parent == null || !(parent instanceof PsiDeclarationStatement)) { - return false; - } - final PsiLocalVariable localVariable = PsiTreeUtil.getPrevSiblingOfType(comment, PsiLocalVariable.class); - if (localVariable == null) { - return false; - } - final PsiNewExpression newExpression = PsiTreeUtil.getChildOfType(localVariable, PsiNewExpression.class); - if (newExpression == null) { - return false; - } - return PsiTreeUtil.getChildOfType(newExpression, PsiAnonymousClass.class) != null; - } - - private static boolean isClassBlockMarker(final PsiElement comment) { - final PsiElement parent = comment.getParent(); - if (parent == null || !(parent instanceof PsiClass)) { - return false; - } - final PsiJavaToken token = PsiTreeUtil.getPrevSiblingOfType(comment, PsiJavaToken.class); - return token != null && JavaTokenType.RBRACE.equals(token.getTokenType()); - } - - private static boolean isTryCatchFinallyBlockMarker(final PsiElement comment) { - final PsiElement parent = comment.getParent(); - if (parent == null || !(parent instanceof PsiTryStatement)) { - return false; - } - return PsiTreeUtil.getPrevSiblingOfType(comment, PsiCodeBlock.class) != null || - PsiTreeUtil.getPrevSiblingOfType(comment, PsiCatchSection.class) != null; - } - - private static boolean isMethodBlockMarker(final PsiElement comment) { - final PsiCodeBlock codeBlock = PsiTreeUtil.getPrevSiblingOfType(comment, PsiCodeBlock.class); - if (codeBlock == null) { - return false; - } - final PsiElement parent = comment.getParent(); - return parent != null && parent instanceof PsiMethod; - } - - private static boolean isLoopOrIfBlockMarker(final PsiElement comment) { - final PsiCodeBlock codeBlock = PsiTreeUtil.getPrevSiblingOfType(comment, PsiCodeBlock.class); - if (codeBlock == null) { - return false; - } - final PsiElement mayBeBlockStatement = comment.getParent(); - if (mayBeBlockStatement == null || !(mayBeBlockStatement instanceof PsiBlockStatement)) { - return false; - } - final PsiElement parent = mayBeBlockStatement.getParent(); - return parent != null && (parent instanceof PsiLoopStatement || parent instanceof PsiIfStatement); - } } diff --git a/java/java-tests/testData/inspection/blockMarkerComments/Class.java b/java/java-tests/testData/inspection/blockMarkerComments/Class.java new file mode 100644 index 000000000000..fdb519f16c64 --- /dev/null +++ b/java/java-tests/testData/inspection/blockMarkerComments/Class.java @@ -0,0 +1,25 @@ +import java.lang.Object; + +class A { + + class Nested { + + } //end class marker + //end not a marker + + void m1() { + Object o = new Object() { + + }; //end anonymous class this is very long comment and it's not marker + //end not a marker + } + + void m() { + Object o; + o = new Object() { + + }; //end marker + } + +} //end marker +//end not a marker \ No newline at end of file diff --git a/java/java-tests/testData/inspection/blockMarkerComments/if/src/Foo.java b/java/java-tests/testData/inspection/blockMarkerComments/If.java similarity index 70% rename from java/java-tests/testData/inspection/blockMarkerComments/if/src/Foo.java rename to java/java-tests/testData/inspection/blockMarkerComments/If.java index 93e7d3c7f349..d43250362499 100644 --- a/java/java-tests/testData/inspection/blockMarkerComments/if/src/Foo.java +++ b/java/java-tests/testData/inspection/blockMarkerComments/If.java @@ -6,11 +6,11 @@ class Foo { } else { - } //endif + } //endif if (true) { - } // end if + } // end if if (true) { diff --git a/java/java-tests/testData/inspection/blockMarkerComments/Loop.java b/java/java-tests/testData/inspection/blockMarkerComments/Loop.java new file mode 100644 index 000000000000..c6e3e21aa316 --- /dev/null +++ b/java/java-tests/testData/inspection/blockMarkerComments/Loop.java @@ -0,0 +1,31 @@ +class Foo { + + void m() { + for (int i = 0; i > -1; i--) { + + } //end + } + + void m1() { + while (true) { + + } + //end while + // (not block marker) + } + + void m2() { + while (true) { + + } // endwhile + } + + void m3() { + do { + + } while (true); + //end + //not a block marker + } + +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/blockMarkerComments/method/src/Foo.java b/java/java-tests/testData/inspection/blockMarkerComments/Method.java similarity index 52% rename from java/java-tests/testData/inspection/blockMarkerComments/method/src/Foo.java rename to java/java-tests/testData/inspection/blockMarkerComments/Method.java index 53a6fcc8bc73..887496db3370 100644 --- a/java/java-tests/testData/inspection/blockMarkerComments/method/src/Foo.java +++ b/java/java-tests/testData/inspection/blockMarkerComments/Method.java @@ -2,7 +2,7 @@ class Foo { void m() { - } // end this is block marker + } // end method void m1() { diff --git a/java/java-tests/testData/inspection/blockMarkerComments/removeBlockMarker.java b/java/java-tests/testData/inspection/blockMarkerComments/RemoveBlockMarker.java similarity index 100% rename from java/java-tests/testData/inspection/blockMarkerComments/removeBlockMarker.java rename to java/java-tests/testData/inspection/blockMarkerComments/RemoveBlockMarker.java diff --git a/java/java-tests/testData/inspection/blockMarkerComments/removeBlockMarker_after.java b/java/java-tests/testData/inspection/blockMarkerComments/RemoveBlockMarker_after.java similarity index 100% rename from java/java-tests/testData/inspection/blockMarkerComments/removeBlockMarker_after.java rename to java/java-tests/testData/inspection/blockMarkerComments/RemoveBlockMarker_after.java diff --git a/java/java-tests/testData/inspection/blockMarkerComments/TryCatch.java b/java/java-tests/testData/inspection/blockMarkerComments/TryCatch.java new file mode 100644 index 000000000000..a692bc7942ef --- /dev/null +++ b/java/java-tests/testData/inspection/blockMarkerComments/TryCatch.java @@ -0,0 +1,45 @@ +import java.lang.Exception; + +class Foo { + + void m() { + + try { + + } catch (Exception e) { + + } //end try-catch + + try { + + } catch (Exception e) { + + } // endtrycatchblockmarker + + try { + + } catch (Exception e) { + + } // endtrycatchblockmarker + + try { + + } catch (Exception e) { + + } finally { + + } // end finally + + try { + + } catch (Exception e) { + + } finally { + + } + // end + // not a block marker + + } + +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/blockMarkerComments/class/expected.xml b/java/java-tests/testData/inspection/blockMarkerComments/class/expected.xml deleted file mode 100644 index 9289d718e726..000000000000 --- a/java/java-tests/testData/inspection/blockMarkerComments/class/expected.xml +++ /dev/null @@ -1,22 +0,0 @@ - - - - Foo.java - 7 - Block marker comment - - - - Foo.java - 17 - Block marker comment - - - - Foo.java - 13 - 0 - Block marker comment - - - \ No newline at end of file diff --git a/java/java-tests/testData/inspection/blockMarkerComments/class/src/Foo.java b/java/java-tests/testData/inspection/blockMarkerComments/class/src/Foo.java deleted file mode 100644 index ac5c4622cf7b..000000000000 --- a/java/java-tests/testData/inspection/blockMarkerComments/class/src/Foo.java +++ /dev/null @@ -1,18 +0,0 @@ -import java.lang.Object; - -class A { - - class Nested { - - } //end class marker - //end not a marker - - void m() { - Object o = new Object() { - - }; //end anonymous class - //end not a marker - } - -} //end marker -//end not a marker \ No newline at end of file diff --git a/java/java-tests/testData/inspection/blockMarkerComments/if/expected.xml b/java/java-tests/testData/inspection/blockMarkerComments/if/expected.xml deleted file mode 100644 index 485f72e34cdd..000000000000 --- a/java/java-tests/testData/inspection/blockMarkerComments/if/expected.xml +++ /dev/null @@ -1,15 +0,0 @@ - - - - Foo.java - 9 - Block marker comment - - - - Foo.java - 13 - Block marker comment - - - \ No newline at end of file diff --git a/java/java-tests/testData/inspection/blockMarkerComments/loop/expected.xml b/java/java-tests/testData/inspection/blockMarkerComments/loop/expected.xml deleted file mode 100644 index 60f286069a42..000000000000 --- a/java/java-tests/testData/inspection/blockMarkerComments/loop/expected.xml +++ /dev/null @@ -1,15 +0,0 @@ - - - - Foo.java - 7 - Block marker comment - - - - Foo.java - 16 - Block marker comment - - - \ No newline at end of file diff --git a/java/java-tests/testData/inspection/blockMarkerComments/loop/src/Foo.java b/java/java-tests/testData/inspection/blockMarkerComments/loop/src/Foo.java deleted file mode 100644 index 538d50b7d098..000000000000 --- a/java/java-tests/testData/inspection/blockMarkerComments/loop/src/Foo.java +++ /dev/null @@ -1,25 +0,0 @@ -class Foo { - - void m() { - - for (int i = 0; i > -1; i--) { - - } //end this is block marker - - while (true) { - - } - //end while (not block marker) - - while () { - - } // end block marker - - do { - - } while (true); - //end, not a block marker - - } - -} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/blockMarkerComments/method/expected.xml b/java/java-tests/testData/inspection/blockMarkerComments/method/expected.xml deleted file mode 100644 index 47492cf6831a..000000000000 --- a/java/java-tests/testData/inspection/blockMarkerComments/method/expected.xml +++ /dev/null @@ -1,9 +0,0 @@ - - - - Foo.java - 5 - Block marker comment - - - \ No newline at end of file diff --git a/java/java-tests/testData/inspection/blockMarkerComments/tryCatch/expected.xml b/java/java-tests/testData/inspection/blockMarkerComments/tryCatch/expected.xml deleted file mode 100644 index 4761b69a9843..000000000000 --- a/java/java-tests/testData/inspection/blockMarkerComments/tryCatch/expected.xml +++ /dev/null @@ -1,27 +0,0 @@ - - - - Foo.java - 11 - Block marker comment - - - - Foo.java - 17 - Block marker comment - - - - Foo.java - 23 - Block marker comment - - - - Foo.java - 31 - Block marker comment - - - \ No newline at end of file diff --git a/java/java-tests/testData/inspection/blockMarkerComments/tryCatch/src/Foo.java b/java/java-tests/testData/inspection/blockMarkerComments/tryCatch/src/Foo.java deleted file mode 100644 index bf1f1442ff22..000000000000 --- a/java/java-tests/testData/inspection/blockMarkerComments/tryCatch/src/Foo.java +++ /dev/null @@ -1,44 +0,0 @@ -import java.lang.Exception; - -class Foo { - - void m() { - - try { - - } catch (Exception e) { - - } //end try-catch block marker - - try { - - } catch (Exception e) { - - } // endtrycatchblockmarker - - try { - - } catch (Exception e) { - - } // endtrycatchblockmarker - - try { - - } catch (Exception e) { - - } finally { - - } // end block marker - - try { - - } catch (Exception e) { - - } finally { - - } - // end not a block marker - - } - -} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/BlockMarkerCommentsTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/BlockMarkerCommentsTest.java index 8ad737256965..0ca40265e4b5 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/BlockMarkerCommentsTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/BlockMarkerCommentsTest.java @@ -17,25 +17,38 @@ package com.intellij.codeInspection; import com.intellij.JavaTestUtil; import com.intellij.codeInsight.intention.IntentionAction; -import com.intellij.codeInspection.ex.LocalInspectionToolWrapper; -import com.intellij.testFramework.fixtures.JavaCodeInsightFixtureTestCase; +import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase; /** * @author Dmitry Batkovich */ -public class BlockMarkerCommentsTest extends JavaCodeInsightFixtureTestCase { +public class BlockMarkerCommentsTest extends LightCodeInsightFixtureTestCase { + + private final BlockMarkerCommentsInspection myInspection = new BlockMarkerCommentsInspection(); @Override protected String getTestDataPath() { return JavaTestUtil.getJavaTestDataPath() + "/inspection/blockMarkerComments/"; } + @Override + public void setUp() throws Exception { + super.setUp(); + myFixture.enableInspections(myInspection); + } + + @Override + public void tearDown() throws Exception { + myFixture.disableInspections(myInspection); + super.tearDown(); + } + private void doTestInspection() { - myFixture.testInspection(getTestName(true), new LocalInspectionToolWrapper(new BlockMarkerCommentsInspection())); + myFixture.testHighlighting(getTestName(false) + ".java"); } private void doTestQuickFix() { - final String testFileName = getTestName(true); + final String testFileName = getTestName(false); myFixture.enableInspections(new BlockMarkerCommentsInspection()); myFixture.configureByFile(testFileName + ".java"); final IntentionAction intentionAction = myFixture.findSingleIntention("Remove block marker comments"); diff --git a/resources/src/META-INF/IdeaPlugin.xml b/resources/src/META-INF/IdeaPlugin.xml index b48be62281c9..18c23b597c72 100644 --- a/resources/src/META-INF/IdeaPlugin.xml +++ b/resources/src/META-INF/IdeaPlugin.xml @@ -660,7 +660,7 @@ displayName="Class may extend a commonly used base class instead of implementing interface"/>