Pull #565: fixed violations of MoveVariableInsideIfCheck

sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/ConfusingConditionCheck.java

@@ -231,9 +231,9 @@ private boolean isRatioBetweenIfAndElseBlockSuitable(DetailAST literalIf) {
         boolean result = true;
 
         final DetailAST lastChildAfterIf = literalIf.getLastChild();
-        final int linesOfCodeInIfBlock = getAmounOfCodeRowsInBlock(literalIf);
         final int linesOfCodeInElseBlock = getAmounOfCodeRowsInBlock(lastChildAfterIf);
         if (linesOfCodeInElseBlock > 0) {
+            final int linesOfCodeInIfBlock = getAmounOfCodeRowsInBlock(literalIf);
             result = linesOfCodeInIfBlock / linesOfCodeInElseBlock < multiplyFactorForElseBlocks;
         }
         return result;

sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/CustomDeclarationOrderCheck.java

@@ -390,6 +390,8 @@ public void visitToken(DetailAST ast) {
     public void leaveToken(DetailAST ast) {
         if (ast.getType() == TokenTypes.CLASS_DEF
                 && !isClassDefInMethodDef(ast)) {
+            // -@cs[MoveVariableInsideIf] assignment value is a modification
+            // call so it can't be moved
             final ClassDetail classDetail = classDetails.pop();
 
             if (checkGettersSetters) {
@@ -687,9 +689,6 @@ private static boolean isSetterName(String methodName) {
     private boolean isGetterCorrect(DetailAST methodDef, String methodPrefix) {
         boolean result = false;
 
-        final String methodName = getIdentifier(methodDef);
-        final String methodNameWithoutPrefix = getNameWithoutPrefix(methodName, methodPrefix);
-
         final DetailAST parameters = methodDef.findFirstToken(TokenTypes.PARAMETERS);
 
         // no parameters
@@ -703,6 +702,9 @@ private boolean isGetterCorrect(DetailAST methodDef, String methodPrefix) {
                 if (returnStatementAst != null) {
                     final DetailAST exprAst = returnStatementAst.getFirstChild();
                     final String returnedFieldName = getNameOfGetterField(exprAst);
+                    final String methodName = getIdentifier(methodDef);
+                    final String methodNameWithoutPrefix = getNameWithoutPrefix(methodName,
+                            methodPrefix);
                     if (returnedFieldName != null
                             && !localVariableHidesField(statementsAst, returnedFieldName)
                             && verifyFieldAndMethodName(returnedFieldName,
@@ -748,15 +750,14 @@ private static boolean localVariableHidesField(DetailAST slist,
     private boolean isSetterCorrect(DetailAST methodDefAst, String methodPrefix) {
         boolean result = false;
 
-        final String methodName = getIdentifier(methodDefAst);
-        final String setterFieldName = fieldPrefix
-                + getNameWithoutPrefix(methodName, methodPrefix);
-
         final DetailAST methodTypeAst = methodDefAst.findFirstToken(TokenTypes.TYPE);
 
         if (methodTypeAst.branchContains(TokenTypes.LITERAL_VOID)) {
 
             final DetailAST statementsAst = methodDefAst.findFirstToken(TokenTypes.SLIST);
+            final String methodName = getIdentifier(methodDefAst);
+            final String setterFieldName = fieldPrefix
+                    + getNameWithoutPrefix(methodName, methodPrefix);
 
             result = statementsAst != null
                     && !localVariableHidesField(statementsAst, setterFieldName)
@@ -1066,9 +1067,8 @@ private static boolean isMainMethodModifiers(final DetailAST methodAST) {
      */
     private static boolean isVoidType(final DetailAST methodAST) {
         boolean result = true;
-        DetailAST methodTypeAST = null;
         if (hasChildToken(methodAST, TokenTypes.TYPE)) {
-            methodTypeAST = methodAST.findFirstToken(TokenTypes.TYPE);
+            final DetailAST methodTypeAST = methodAST.findFirstToken(TokenTypes.TYPE);
             result = hasChildToken(methodTypeAST, TokenTypes.LITERAL_VOID);
         }
         return result;
@@ -1343,9 +1343,8 @@ else if (isBooleanGetterName(getterName)) {
                         // method is NOT getter
                         final DetailAST setterAst = allGettersSetters.get(j);
                         final String setterName = getIdentifier(setterAst);
-                        String setterField = null;
                         if (isSetterName(setterName)) {
-                            setterField = getNameWithoutPrefix(
+                            final String setterField = getNameWithoutPrefix(
                                     getIdentifier(setterAst), SETTER_PREFIX);
 
                             // if fields are same and setter is sibling with getter

sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/MapIterationInForEachLoopCheck.java

@@ -330,14 +330,10 @@ private String validate(DetailAST forLiteralNode) {
         final DetailAST forEachNode = forLiteralNode.findFirstToken(TokenTypes.FOR_EACH_CLAUSE);
         final DetailAST keySetOrEntrySetNode =
                 getKeySetOrEntrySetNode(forEachNode);
-        boolean isMapClassField = false;
         // Search for keySet or entrySet
         if (keySetOrEntrySetNode != null) {
-            if (keySetOrEntrySetNode.getPreviousSibling().getChildCount() != 0) {
-                isMapClassField = true;
-            }
-            final DetailAST variableDefNode = forEachNode.getFirstChild();
-            final String keyOrEntryVariableName = variableDefNode.getLastChild().getText();
+            final boolean isMapClassField = keySetOrEntrySetNode.getPreviousSibling()
+                    .getChildCount() != 0;
 
             final String currentMapVariableName;
 
@@ -351,6 +347,8 @@ private String validate(DetailAST forLiteralNode) {
             final DetailAST forEachOpeningBrace = forLiteralNode.getLastChild();
 
             if (!isMapPassedIntoAnyMethod(forEachOpeningBrace)) {
+                final DetailAST variableDefNode = forEachNode.getFirstChild();
+                final String keyOrEntryVariableName = variableDefNode.getLastChild().getText();
 
                 if (proposeKeySetUsage
                         && KEY_SET_METHOD_NAME.equals(

sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/MultipleVariableDeclarationsExtendedCheck.java

@@ -98,10 +98,12 @@ public int[] getDefaultTokens() {
     public void work(DetailAST ast) {
 
         DetailAST nextNode = ast.getNextSibling();
-        final boolean isCommaSeparated = (nextNode != null)
-                && (nextNode.getType() == TokenTypes.COMMA);
 
         if (nextNode != null) {
+            // -@cs[MoveVariableInsideIf] assignment value is modified later so
+            // it can't be moved
+            final boolean isCommaSeparated = nextNode.getType() == TokenTypes.COMMA;
+
             if ((nextNode.getType() == TokenTypes.COMMA)
                     || (nextNode.getType() == TokenTypes.SEMI)) {
                 nextNode = nextNode.getNextSibling();

sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/SimpleAccessorNameNotationCheck.java

@@ -93,8 +93,8 @@ public int[] getDefaultTokens() {
 
     @Override
     public void visitToken(DetailAST methodDef) {
-        final String methodName = methodDef.findFirstToken(TokenTypes.IDENT).getText();
         if (hasBody(methodDef) && !isMethodAtAnonymousClass(methodDef)) {
+            final String methodName = methodDef.findFirstToken(TokenTypes.IDENT).getText();
             if (methodName.startsWith(BOOLEAN_GETTER_PREFIX)) {
                 if (!isGetterCorrect(methodDef,
                         methodName.substring(BOOLEAN_GETTER_PREFIX.length()))) {
@@ -224,10 +224,9 @@ private static String getNameOfSettingField(DetailAST assign,
             DetailAST parameters) {
         String nameOfSettingField = null;
 
-        final DetailAST assigningFirstChild = assign.getFirstChild();
-
         if (assign.getChildCount() == 2
                 && assign.getLastChild().getType() == TokenTypes.IDENT) {
+            final DetailAST assigningFirstChild = assign.getFirstChild();
 
             if (assigningFirstChild.getType() == TokenTypes.IDENT) {
 

sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/design/PublicReferenceToPrivateTypeCheck.java

@@ -214,11 +214,11 @@ private void addExternallyAccessibleFieldTypes(DetailAST fieldDefAst) {
      */
     private static List<DetailAST>
             getMethodParameterTypes(DetailAST parametersDefAst) {
-        DetailAST parameterType = null;
         final List<DetailAST> parameterTypes = new ArrayList<>();
 
         if (parametersDefAst.getFirstChild() != null) {
             DetailAST currentNode = parametersDefAst;
+            DetailAST parameterType = null;
 
             while (currentNode != null) {
                 if (currentNode.getType() == TokenTypes.PARAMETER_DEF) {

sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/sizes/LineLengthExtendedCheck.java

@@ -194,11 +194,12 @@ public int[] getDefaultTokens() {
 
     @Override
     public void visitToken(DetailAST ast) {
-        final DetailAST endOfIgnoreLine = ast.findFirstToken(TokenTypes.SLIST);
         if (ast.getParent() != null
                 && ast.getParent().getType() == TokenTypes.OBJBLOCK
                 || ast.getType() == TokenTypes.CLASS_DEF) {
             final int mNumberOfLine = ast.getLineNo();
+            final DetailAST endOfIgnoreLine = ast.findFirstToken(TokenTypes.SLIST);
+
             if (endOfIgnoreLine == null) {
                 lines[mNumberOfLine - 1] = null;
             }

sevntu-checks/src/test/java/com/github/sevntu/checkstyle/internal/CheckstyleRegressionTest.java

@@ -88,9 +88,6 @@ private static void work(File config, File suppression) throws Exception {
 
         trimSevntuChecksNotReferenced(configChecks, sevntuChecks);
 
-        String configContents = new String(Files.readAllBytes(config.toPath()), UTF_8);
-        String suppressionContents = new String(Files.readAllBytes(suppression.toPath()), UTF_8);
-
         if (sevntuChecks.size() > 0) {
             System.out.println("Adding " + sevntuChecks.size() + " missing check(s)");
 
@@ -113,6 +110,8 @@ private static void work(File config, File suppression) throws Exception {
                 }
             }
 
+            String configContents = new String(Files.readAllBytes(config.toPath()), UTF_8);
+
             int treeWalkerPosition = configContents.lastIndexOf("<module name=\"TreeWalker\">");
             treeWalkerPosition = configContents.indexOf('\n', treeWalkerPosition) + 1;
 
@@ -122,6 +121,7 @@ private static void work(File config, File suppression) throws Exception {
             Files.write(config.toPath(), configContents.getBytes(UTF_8), StandardOpenOption.CREATE);
 
             if (!suppressionAdditions.isEmpty()) {
+                String suppressionContents = new String(Files.readAllBytes(suppression.toPath()), UTF_8);
                 final int position = suppressionContents.lastIndexOf("</suppressions");
 
                 suppressionContents = suppressionContents.substring(0, position)

sevntu-checks/src/test/java/com/github/sevntu/checkstyle/internal/CommitValidationTest.java

@@ -134,11 +134,11 @@ public void testCommitMessage() {
     @Test
     public void testCommitMessageHasProperStructure() {
         for (RevCommit commit : filterValidCommits(lastCommits)) {
-            final String commitId = commit.getId().getName();
             final String commitMessage = commit.getFullMessage();
             final int error = validateCommitMessage(commitMessage);
 
             if (error != 0) {
+                final String commitId = commit.getId().getName();
                 fail(getInvalidCommitMessageFormattingError(commitId, commitMessage) + error);
             }
         }