From 4a0ee1de46ae523588f55e6becfeae131fe0b8d4 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Sun, 28 May 2017 19:56:43 +0100 Subject: [PATCH] ruleset: MoveVariableInsideIf: enabled for 4-tweaks and above --- README.md | 24 +++++++ builder/src/main/resources/application.yml | 7 ++ .../rules/MoveVariableInsideIfCheck.md | 22 +++++++ .../regressions/MoveVariableInsideIf.java | 65 +++++++++++++++++++ .../net/kemitix/checkstyle-4-tweaks.xml | 1 + .../net/kemitix/checkstyle-5-complexity.xml | 1 + 6 files changed, 120 insertions(+) create mode 100644 builder/src/main/resources/rules/MoveVariableInsideIfCheck.md create mode 100644 regressions/src/main/java/net/kemitix/checkstyle/regressions/MoveVariableInsideIf.java diff --git a/README.md b/README.md index 9cdfee9..ac39463 100644 --- a/README.md +++ b/README.md @@ -181,6 +181,7 @@ Rule|Level|Source|Enabled|Suppressible [MissingSwitchDefault](#missingswitchdefault)|tweaks|checkstyle|Yes| [ModifiedControlVariable](#modifiedcontrolvariable)|tweaks|checkstyle|Yes| [ModifierOrder](#modifierorder)|naming|checkstyle|Yes| +[MoveVariableInsideIfCheck](#movevariableinsideifcheck)|tweaks|sevntu|Yes| [MultipleStringLiterals](#multiplestringliterals)|naming|checkstyle|Yes| [MultipleVariableDeclarations](#multiplevariabledeclarations)|naming|checkstyle|Yes| [MutableException](#mutableexception)|tweaks|checkstyle|Yes| @@ -2333,6 +2334,29 @@ if (getProperty() && property) {} #### [MapIterationInForEachLoop](http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/coding/MapIterationInForEachLoopCheck.html) Checks for unoptimised iterations over `Map`s. Check use of `map.values()`, `map.keySet()` and `map.entrySet()` against the use of the iterator produced to verify if another could be better. +#### [MoveVariableInsideIfCheck](http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/coding/MoveVariableInsideIfCheck.html) + +Checks if a variable is declared outside an `if` block that is only used within that block. + +Valid: +```` +if (condition) { + String variable = input.substring(1); + return method(variable); +} +return ""; +```` + +Invalid: +```` +String variable = input.substring(1); +if (condition) { + return method(variable); +} +return ""; +```` + +Example: [MoveVariableInsideIf.java](https://github.com/kemitix/kemitix-checkstyle-ruleset/blob/master/regressions/src/main/java/net/kemitix/checkstyle/regressions/MoveVariableInsideIf.java) #### [NameConventionForJunit4TestClasses](http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/coding/NameConventionForJunit4TestClassesCheck.html) Checks the names of JUnit test classes. Classes checked are those that have at least one method annotated with `Test` or `org.junit.Test`. diff --git a/builder/src/main/resources/application.yml b/builder/src/main/resources/application.yml index 7519e16..3b60a5d 100644 --- a/builder/src/main/resources/application.yml +++ b/builder/src/main/resources/application.yml @@ -1498,3 +1498,10 @@ rules: source: SEVNTU uri: http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/coding/WhitespaceBeforeArrayInitializerCheck.html reason: "TODO: enable" + - + name: MoveVariableInsideIfCheck + parent: TREEWALKER + level: TWEAKS + enabled: true + source: SEVNTU + uri: http://sevntu-checkstyle.github.io/sevntu.checkstyle/apidocs/com/github/sevntu/checkstyle/checks/coding/MoveVariableInsideIfCheck.html diff --git a/builder/src/main/resources/rules/MoveVariableInsideIfCheck.md b/builder/src/main/resources/rules/MoveVariableInsideIfCheck.md new file mode 100644 index 0000000..e5304b3 --- /dev/null +++ b/builder/src/main/resources/rules/MoveVariableInsideIfCheck.md @@ -0,0 +1,22 @@ + +Checks if a variable is declared outside an `if` block that is only used within that block. + +Valid: +```` +if (condition) { + String variable = input.substring(1); + return method(variable); +} +return ""; +```` + +Invalid: +```` +String variable = input.substring(1); +if (condition) { + return method(variable); +} +return ""; +```` + +Example: [MoveVariableInsideIf.java](https://github.com/kemitix/kemitix-checkstyle-ruleset/blob/master/regressions/src/main/java/net/kemitix/checkstyle/regressions/MoveVariableInsideIf.java) diff --git a/regressions/src/main/java/net/kemitix/checkstyle/regressions/MoveVariableInsideIf.java b/regressions/src/main/java/net/kemitix/checkstyle/regressions/MoveVariableInsideIf.java new file mode 100644 index 0000000..bfdb49d --- /dev/null +++ b/regressions/src/main/java/net/kemitix/checkstyle/regressions/MoveVariableInsideIf.java @@ -0,0 +1,65 @@ +/** + * The MIT License (MIT) + * + * Copyright (c) 2017 Paul Campbell + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of this software + * and associated documentation files (the "Software"), to deal in the Software without restriction, + * including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all copies + * or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, + * INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE + * AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +package net.kemitix.checkstyle.regressions; + +/** + * Regression demo for {@code MoveVariableInsideIfCheck}. + * + * @author Paul Campbell (pcampbell@kemitix.net) + */ +class MoveVariableInsideIf { + + private String input = "1"; + + private boolean condition; + + private String method(final String variable) { + return "value"; + } + + /** + * Fails if not suppressed. + * + * @return value + */ + @SuppressWarnings("movevariableinsideif") + String invalid() { + String variable = input.substring(1); + if (condition) { + return method(variable); + } + return ""; + } + + /** + * Rewrite {@link #invalid()} as this to pass. + * + * @return value + */ + String valid() { + if (condition) { + String variable = input.substring(1); + return method(variable); + } + return ""; + } +} diff --git a/ruleset/src/main/resources/net/kemitix/checkstyle-4-tweaks.xml b/ruleset/src/main/resources/net/kemitix/checkstyle-4-tweaks.xml index 50b1a88..2e27c90 100644 --- a/ruleset/src/main/resources/net/kemitix/checkstyle-4-tweaks.xml +++ b/ruleset/src/main/resources/net/kemitix/checkstyle-4-tweaks.xml @@ -175,6 +175,7 @@ + diff --git a/ruleset/src/main/resources/net/kemitix/checkstyle-5-complexity.xml b/ruleset/src/main/resources/net/kemitix/checkstyle-5-complexity.xml index b258137..bc2b68b 100644 --- a/ruleset/src/main/resources/net/kemitix/checkstyle-5-complexity.xml +++ b/ruleset/src/main/resources/net/kemitix/checkstyle-5-complexity.xml @@ -228,6 +228,7 @@ +