From dcef98fe6e27a960de45bb352c2beb9edb7135f0 Mon Sep 17 00:00:00 2001 From: Pavel Dolgov Date: Mon, 17 Jul 2017 17:08:03 +0300 Subject: [PATCH] Java: Detect potential dangling 'else' in "code block contains single statement" inspection (IDEA-174415) --- .../SingleStatementInBlockInspection.java | 41 ++++- .../SingleStatement.java | 63 ------- .../style/single_statement_block/expected.xml | 102 ----------- .../SingleStatement.java | 169 ++++++++++++++++++ .../SingleStatementInBlockInspectionTest.java | 14 +- 5 files changed, 219 insertions(+), 170 deletions(-) delete mode 100644 plugins/InspectionGadgets/test/com/siyeh/igtest/style/single_statement_block/SingleStatement.java delete mode 100644 plugins/InspectionGadgets/test/com/siyeh/igtest/style/single_statement_block/expected.xml create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igtest/style/single_statement_in_block/SingleStatement.java diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/style/SingleStatementInBlockInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/style/SingleStatementInBlockInspection.java index 1b8163f04e23..41802581d029 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/style/SingleStatementInBlockInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/style/SingleStatementInBlockInspection.java @@ -98,7 +98,7 @@ public class SingleStatementInBlockInspection extends BaseInspection { protected boolean isApplicable(PsiStatement body) { if (body instanceof PsiBlockStatement) { final PsiStatement[] statements = ((PsiBlockStatement)body).getCodeBlock().getStatements(); - if (statements.length == 1 && !(statements[0] instanceof PsiDeclarationStatement)) { + if (statements.length == 1 && !(statements[0] instanceof PsiDeclarationStatement) && !isDanglingElseProblem(statements[0], body)) { final PsiFile file = body.getContainingFile(); //this inspection doesn't work in JSP files, as it can't tell about tags // inside the braces @@ -125,6 +125,45 @@ public class SingleStatementInBlockInspection extends BaseInspection { } return null; } + + /** + * See JLS paragraphs 14.5, 14.9 + */ + private static boolean isDanglingElseProblem(@Nullable PsiStatement statement, @NotNull PsiStatement outerStatement) { + return hasShortIf(statement) && hasPotentialDanglingElse(outerStatement); + } + + private static boolean hasShortIf(@Nullable PsiStatement statement) { + if (statement instanceof PsiIfStatement) { + final PsiStatement elseBranch = ((PsiIfStatement)statement).getElseBranch(); + return elseBranch == null || hasShortIf(elseBranch); + } + if (statement instanceof PsiLabeledStatement) { + return hasShortIf(((PsiLabeledStatement)statement).getStatement()); + } + if (statement instanceof PsiWhileStatement || statement instanceof PsiForStatement || statement instanceof PsiForeachStatement) { + return hasShortIf(((PsiLoopStatement)statement).getBody()); + } + return false; + } + + private static boolean hasPotentialDanglingElse(@NotNull PsiStatement statement) { + final PsiElement parent = statement.getParent(); + if (parent instanceof PsiIfStatement) { + final PsiIfStatement ifStatement = (PsiIfStatement)parent; + if (ifStatement.getThenBranch() == statement && ifStatement.getElseBranch() != null) { + return true; + } + return hasPotentialDanglingElse(ifStatement); + } + if (parent instanceof PsiLabeledStatement || + parent instanceof PsiWhileStatement || + parent instanceof PsiForStatement || + parent instanceof PsiForeachStatement) { + return hasPotentialDanglingElse((PsiStatement)parent); + } + return false; + } } private static class SingleStatementInBlockFix extends InspectionGadgetsFix { diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/style/single_statement_block/SingleStatement.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/style/single_statement_block/SingleStatement.java deleted file mode 100644 index 12af1bec3acf..000000000000 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/style/single_statement_block/SingleStatement.java +++ /dev/null @@ -1,63 +0,0 @@ -class T { - - void f(String[] a) { - for (String s : a) { - System.out.println(s); - } - - if (a.length == 0) { - System.out.println("no"); - } else { - System.out.println(a.length); - } - - if (a.length == 0) { - System.out.println("no"); - } - - if (a.length == 0) { - } else { - System.out.println(a.length); - } - - for (int i = 0; i < a.length; i++) { - System.out.println(a[i]); - } - - int j = 0; - do { - System.out.println(a[j++]); - } - while (j < a.length); - - int k = 0; - while (k < a.length) { - System.out.println(a[k++]); - } - } - - void ff(String[] a) { - if (a.length != 0) { - for (String arg : a) { - if (arg.length() > 1) { - for (int i = 0; i < arg.length(); i++) { - System.out.println(arg.charAt(i)); - } - } else { - System.out.println(0); - } - } - } else { - System.out.println("no"); - } - } - - void decl(String[] a) { - if (a.length == 1) { - String t = a[0]; - } - for (int i = 0; i < a.length; i++) { - String t = a[i]; - } - } -} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/style/single_statement_block/expected.xml b/plugins/InspectionGadgets/test/com/siyeh/igtest/style/single_statement_block/expected.xml deleted file mode 100644 index 9d1103579b2d..000000000000 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/style/single_statement_block/expected.xml +++ /dev/null @@ -1,102 +0,0 @@ - - - - - SingleStatement.java - 4 - Code block contains single statement - 'for' contains single statement - - - - SingleStatement.java - 8 - Code block contains single statement - 'if' contains single statement - - - - SingleStatement.java - 10 - Code block contains single statement - 'else' contains single statement - - - - SingleStatement.java - 14 - Code block contains single statement - 'if' contains single statement - - - - SingleStatement.java - 19 - Code block contains single statement - 'else' contains single statement - - - - SingleStatement.java - 23 - Code block contains single statement - 'for' contains single statement - - - - SingleStatement.java - 28 - Code block contains single statement - 'do' contains single statement - - - - SingleStatement.java - 34 - Code block contains single statement - 'while' contains single statement - - - - SingleStatement.java - 40 - Code block contains single statement - 'if' contains single statement - - - - SingleStatement.java - 41 - Code block contains single statement - 'for' contains single statement - - - - SingleStatement.java - 42 - Code block contains single statement - 'if' contains single statement - - - - SingleStatement.java - 43 - Code block contains single statement - 'for' contains single statement - - - - SingleStatement.java - 46 - Code block contains single statement - 'else' contains single statement - - - - SingleStatement.java - 50 - Code block contains single statement - 'else' contains single statement - - - \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/style/single_statement_in_block/SingleStatement.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/style/single_statement_in_block/SingleStatement.java new file mode 100644 index 000000000000..cf8f1a747d3a --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/style/single_statement_in_block/SingleStatement.java @@ -0,0 +1,169 @@ +class T { + + void f(String[] a) { + for (String s : a) { + System.out.println(s); + } + + if (a.length == 0) { + System.out.println("no"); + } else { + System.out.println(a.length); + } + + if (a.length == 0) { + System.out.println("no"); + } + + if (a.length == 0) { + } else { + System.out.println(a.length); + } + + for (int i = 0; i < a.length; i++) { + System.out.println(a[i]); + } + + int j = 0; + do { + System.out.println(a[j++]); + } + while (j < a.length); + + int k = 0; + while (k < a.length) { + System.out.println(a[k++]); + } + } + + void nested(String[] a) { + if (a.length != 0) { + for (String arg : a) { + if (arg.length() > 1) { + for (int i = 0; i < arg.length(); i++) { + System.out.println(arg.charAt(i)); + } + } else { + System.out.println(0); + } + } + } else { + System.out.println("no"); + } + } + + void decl(String[] a) { + if (a.length == 1) { + String t = a[0]; + } + for (int i = 0; i < a.length; i++) { + String t = a[i]; + } + } + + void labeled(String[] a) { + OuterIf: + if (a != null) { + OuterFor: + for (String s : a) { + InnerFor: + for (int i = 0; i < s.length(); i++) { + InnerIf: + if (s.charAt(i) == ' ') { + break OuterFor; + } + } + } + } + } + + void danglingElse(Object[] a) { + if (a != null) { + if (a.length != 0) + System.out.println(a[0]); + } + else + System.out.println("null"); + } + + void noDanglingElse(Object[] a) { + if (a != null) { + if (a.length != 0) + System.out.println(a[0]); + else + System.out.println("empty"); + } + else + System.out.println("null"); + } + + void danglingElseNestedIfChain(Object[] a) { + if (a != null) { + if (a.length != 0) + if(a[0] != null) + System.out.println(a[0]); + } + else + System.out.println("null"); + } + + void danglingElseNestedIfElse(Object[] a) { + if (a != null) { + if (a.length != 0) + if (a[0] != null) + System.out.println(a[0]); + else + System.out.println("missing"); + } + else + System.out.println("null"); + } + + void noDanglingElseNestedIf(Object[] a) { + if (a != null) { + if (a.length != 0) + if (a[0] != null) + System.out.println(a[0]); + else + System.out.println("missing"); + else + System.out.println("empty"); + } + else + System.out.println("null"); + } + + void danglingElseWithLoop(Object[] a) { + if (a != null) { + for (int i = 0; i < a.length; i++) + if (a[i] != null) + System.out.println(a[i]); + } + else + System.out.println("null"); + } + + void noDanglingElseWithLoop(Object[] a) { + if (a != null) { + for (int i = 0; i < a.length; i++) + if (a[i] != null) + System.out.println(a[i]); + else + System.out.println("missing"); + } + else + System.out.println("null"); + } + + public int danglingElseWithTwoLoops(Object[] a, Object o) { + if (o == null) { + for (int i = 0; i < a.length; i++) + if (a[i] == null) + return i; + } else + for (int i = 0; i < a.length; i++) + if (o.equals(a[i])) + return i; + return -1; + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/style/SingleStatementInBlockInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/style/SingleStatementInBlockInspectionTest.java index a2f7defa9802..1afba35e74ff 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/style/SingleStatementInBlockInspectionTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/style/SingleStatementInBlockInspectionTest.java @@ -15,13 +15,19 @@ */ package com.siyeh.ig.style; -import com.siyeh.ig.IGInspectionTestCase; +import com.intellij.codeInspection.InspectionProfileEntry; +import com.siyeh.ig.LightInspectionTestCase; /** * @author Pavel.Dolgov */ -public class SingleStatementInBlockInspectionTest extends IGInspectionTestCase { - public void test() { - doTest("com/siyeh/igtest/style/single_statement_block", new SingleStatementInBlockInspection()); +public class SingleStatementInBlockInspectionTest extends LightInspectionTestCase { + public void testSingleStatement() { + doTest(); + } + + @Override + protected InspectionProfileEntry getInspection() { + return new SingleStatementInBlockInspection(); } }