BlockMarkerCommentsInspection changes after review

This commit is contained in:
Dmitry Batkovich
2014-05-29 18:04:04 +04:00
parent 98954e2ee6
commit 2d0aa2ae3a
18 changed files with 178 additions and 255 deletions
@@ -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<PsiJavaToken>(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<PsiJavaToken>(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);
}
}
@@ -0,0 +1,25 @@
import java.lang.Object;
class A {
class Nested {
} <warning descr="Redundant block marker">//end class marker</warning>
//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() {
}; <warning descr="Redundant block marker">//end marker</warning>
}
} <warning descr="Redundant block marker">//end marker</warning>
//end not a marker
@@ -6,11 +6,11 @@ class Foo {
} else {
} //endif
} <warning descr="Redundant block marker">//endif</warning>
if (true) {
} // end if
} <warning descr="Redundant block marker">// end if</warning>
if (true) {
@@ -0,0 +1,31 @@
class Foo {
void m() {
for (int i = 0; i > -1; i--) {
} <warning descr="Redundant block marker">//end</warning>
}
void m1() {
while (true) {
}
//end while
// (not block marker)
}
void m2() {
while (true) {
} <warning descr="Redundant block marker">// endwhile</warning>
}
void m3() {
do {
} while (true);
//end
//not a block marker
}
}
@@ -2,7 +2,7 @@ class Foo {
void m() {
} // end this is block marker
} <warning descr="Redundant block marker">// end method</warning>
void m1() {
@@ -0,0 +1,45 @@
import java.lang.Exception;
class Foo {
void m() {
try {
} catch (Exception e) {
} <warning descr="Redundant block marker">//end try-catch</warning>
try {
} catch (Exception e) {
} <warning descr="Redundant block marker">// endtrycatchblockmarker</warning>
try {
} catch (Exception e) {
} <warning descr="Redundant block marker">// endtrycatchblockmarker</warning>
try {
} catch (Exception e) {
} finally {
} <warning descr="Redundant block marker">// end finally</warning>
try {
} catch (Exception e) {
} finally {
}
// end
// not a block marker
}
}
@@ -1,22 +0,0 @@
<?xml version="1.0" encoding="UTF-8"?>
<problems>
<problem>
<file>Foo.java</file>
<line>7</line>
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Block marker comment</problem_class>
<description />
</problem>
<problem>
<file>Foo.java</file>
<line>17</line>
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Block marker comment</problem_class>
<description />
</problem>
<problem>
<file>Foo.java</file>
<line>13</line>
<module>0</module>
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Block marker comment</problem_class>
<description />
</problem>
</problems>
@@ -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
@@ -1,15 +0,0 @@
<?xml version="1.0" encoding="UTF-8"?>
<problems>
<problem>
<file>Foo.java</file>
<line>9</line>
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Block marker comment</problem_class>
<description/>
</problem>
<problem>
<file>Foo.java</file>
<line>13</line>
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Block marker comment</problem_class>
<description/>
</problem>
</problems>
@@ -1,15 +0,0 @@
<?xml version="1.0" encoding="UTF-8"?>
<problems>
<problem>
<file>Foo.java</file>
<line>7</line>
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Block marker comment</problem_class>
<description />
</problem>
<problem>
<file>Foo.java</file>
<line>16</line>
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Block marker comment</problem_class>
<description />
</problem>
</problems>
@@ -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
}
}
@@ -1,9 +0,0 @@
<?xml version="1.0" encoding="UTF-8"?>
<problems>
<problem>
<file>Foo.java</file>
<line>5</line>
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Block marker comment</problem_class>
<description />
</problem>
</problems>
@@ -1,27 +0,0 @@
<?xml version="1.0" encoding="UTF-8"?>
<problems>
<problem>
<file>Foo.java</file>
<line>11</line>
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Block marker comment</problem_class>
<description />
</problem>
<problem>
<file>Foo.java</file>
<line>17</line>
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Block marker comment</problem_class>
<description />
</problem>
<problem>
<file>Foo.java</file>
<line>23</line>
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Block marker comment</problem_class>
<description />
</problem>
<problem>
<file>Foo.java</file>
<line>31</line>
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Block marker comment</problem_class>
<description />
</problem>
</problems>
@@ -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
}
}
@@ -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");
+1 -1
View File
@@ -660,7 +660,7 @@
displayName="Class may extend a commonly used base class instead of implementing interface"/>
<localInspection language="JAVA" shortName="BlockMarkerComments"
groupBundle="messages.InspectionsBundle"
groupName="Code style issues" enabledByDefault="true" level="WARNING"
groupName="Code style issues" enabledByDefault="false" level="WARNING"
implementationClass="com.intellij.codeInspection.BlockMarkerCommentsInspection"
displayName="Block marker comment"/>