refacto(checks): factorizing check methods like end of file or method… (

#35) * refacto(checks): factorizing check methods like end of file or method call, to improve readability * fix(geoloc): added missing test case on geoloc accuracy management
green-code-initiative · May 30, 2024 · 98cd1c3 · 98cd1c3
1 parent 4faee7b
commit 98cd1c3
Show file tree

Hide file tree

Showing 16 changed files with 145 additions and 70 deletions.
diff --git a/swift-lang/src/main/java/io/ecocode/ios/swift/checks/CheckHelper.java b/swift-lang/src/main/java/io/ecocode/ios/swift/checks/CheckHelper.java
@@ -19,12 +19,58 @@
 
 import io.ecocode.ios.swift.antlr.generated.Swift5Parser;
 import org.antlr.v4.runtime.tree.ParseTree;
+import org.antlr.v4.runtime.tree.TerminalNodeImpl;
 
 public class CheckHelper {
+    private static final String METHOD_CALL_PATTERN = ".%s(";
+    public static final String END_OF_FILE = "<EOF>";
+
     private CheckHelper() {
     }
 
+    /**
+     * Utility method to check if the given import is present in the parse tree
+     *
+     * @param tree       the parse tree to check
+     * @param importName the import name to check for
+     * @return true if the import is effectively present in the parse tree
+     */
     public static boolean isImportExisting(ParseTree tree, String importName) {
-        return (tree instanceof Swift5Parser.Import_declarationContext && tree.getText().contains(importName));
+        return (tree instanceof Swift5Parser.Import_declarationContext &&
+                tree.getText().contains(importName)
+        );
+    }
+
+    /**
+     * Utility method to check if a call to the given method or function is present in the parse tree
+     *
+     * @param tree       the parse tree to check
+     * @param methodName the method name to check for call
+     * @return true if the method is effectively called in the parse tree
+     */
+    public static boolean isFunctionCalled(ParseTree tree, String methodName) {
+        return (tree instanceof Swift5Parser.ExpressionContext &&
+                tree.getText().contains(String.format(METHOD_CALL_PATTERN, methodName)));
+    }
+    /**
+     * Utility method to check if the given expression is present in the parse tree
+     *
+     * @param tree       the parse tree to check
+     * @param expression the expression to check for presence
+     * @return true if the expression is effectively used in the parse tree
+     */
+    public static boolean isExpressionPresent(ParseTree tree, String expression) {
+        return (tree instanceof Swift5Parser.ExpressionContext &&
+                tree.getText().contains(expression));
+    }
+
+    /**
+     * Utility method to check if the parse tree represents the end of the analyzed file
+     *
+     * @param tree the parse tree to check
+     * @return true if the end of file is reached within the parse tree
+     */
+    public static boolean isEndOfFile(ParseTree tree){
+        return tree instanceof TerminalNodeImpl && tree.getText().equals(END_OF_FILE);
     }
 }
diff --git a/swift-lang/src/main/java/io/ecocode/ios/swift/checks/camera/CameraLeakCheck.java b/swift-lang/src/main/java/io/ecocode/ios/swift/checks/camera/CameraLeakCheck.java
@@ -23,6 +23,9 @@
 import org.antlr.v4.runtime.tree.TerminalNodeImpl;
 import org.sonar.check.Rule;
 
+import static io.ecocode.ios.swift.checks.CheckHelper.isEndOfFile;
+import static io.ecocode.ios.swift.checks.CheckHelper.isExpressionPresent;
+
 @Rule(key = "EC512")
 public class CameraLeakCheck extends SwiftRuleCheck {
     private static final String DEFAULT_ISSUE_MESSAGE = "Any started capture session should be stopped.";
@@ -32,17 +35,16 @@ public class CameraLeakCheck extends SwiftRuleCheck {
 
     @Override
     public void apply(ParseTree tree) {
-        if (tree instanceof Swift5Parser.ExpressionContext && tree.getText().contains("startRunning")) {
+        if (isExpressionPresent(tree,"startRunning")) {
             id = (Swift5Parser.ExpressionContext) tree;
             captureSessionStarted = true;
         }
 
-        if (tree instanceof Swift5Parser.ExpressionContext
-                && (tree.getText().contains("stopRunning"))) {
+        if (isExpressionPresent(tree,"stopRunning")) {
             captureSessionStopped = true;
         }
 
-        if (tree instanceof TerminalNodeImpl && tree.getText().equals("<EOF>")) {
+        if (isEndOfFile(tree)) {
             if (captureSessionStarted && !captureSessionStopped) {
                 this.recordIssue(id.getStart().getStartIndex(), DEFAULT_ISSUE_MESSAGE);
             }

diff --git a/swift-lang/src/main/java/io/ecocode/ios/swift/checks/geolocalisation/ThriftyGeolocation.java b/swift-lang/src/main/java/io/ecocode/ios/swift/checks/geolocalisation/ThriftyGeolocation.java
@@ -23,7 +23,7 @@
 import org.antlr.v4.runtime.tree.TerminalNodeImpl;
 import org.sonar.check.Rule;
 
-import static io.ecocode.ios.swift.checks.CheckHelper.isImportExisting;
+import static io.ecocode.ios.swift.checks.CheckHelper.*;
 
 @Rule(key = "EC524")
 public class ThriftyGeolocation extends SwiftRuleCheck {
@@ -39,13 +39,11 @@ public void apply(ParseTree tree) {
             importExist = true;
         }
 
-        if (!geolocationUpdated
-            && tree instanceof Swift5Parser.ExpressionContext
-            && (tree.getText().contains("desiredAccuracy") || tree.getText().contains("CLActivityType"))) {
-            geolocationUpdated = true;
-        }
+        geolocationUpdated = geolocationUpdated ||
+                (isExpressionPresent(tree, "desiredAccuracy") ||
+                isExpressionPresent(tree, "CLActivityType"));
 
-        if (tree instanceof TerminalNodeImpl && tree.getText().equals("<EOF>")) {
+        if (isEndOfFile(tree)) {
             if (importExist && !geolocationUpdated) {
                 this.recordIssue(importTree.getStart().getStartIndex(), DEFAULT_ISSUE_MESSAGE);
             }

diff --git a/swift-lang/src/main/java/io/ecocode/ios/swift/checks/idleness/IdleTimerDisabledCheck.java b/swift-lang/src/main/java/io/ecocode/ios/swift/checks/idleness/IdleTimerDisabledCheck.java
@@ -22,6 +22,8 @@
 import org.antlr.v4.runtime.tree.ParseTree;
 import org.sonar.check.Rule;
 
+import static io.ecocode.ios.swift.checks.CheckHelper.isExpressionPresent;
+
 /**
  * Check the use of "UIApplication.shared.isIdleTimerDisabled" and triggers when set to true.
  */
@@ -31,12 +33,9 @@ public class IdleTimerDisabledCheck extends SwiftRuleCheck {
 
     @Override
     public void apply(ParseTree tree) {
-
-        if (tree instanceof Swift5Parser.ExpressionContext) {
+        if (isExpressionPresent(tree,"UIApplication.shared.isIdleTimerDisabled=true")) {
             Swift5Parser.ExpressionContext id = (Swift5Parser.ExpressionContext) tree;
-            if (id.getText().equals("UIApplication.shared.isIdleTimerDisabled=true")) {
-                this.recordIssue(id.getStart().getStartIndex(), DEFAULT_ISSUE_MESSAGE);
-            }
+            this.recordIssue(id.getStart().getStartIndex(), DEFAULT_ISSUE_MESSAGE);
         }
     }
 }
diff --git a/swift-lang/src/main/java/io/ecocode/ios/swift/checks/idleness/RigidAlarmCheck.java b/swift-lang/src/main/java/io/ecocode/ios/swift/checks/idleness/RigidAlarmCheck.java
@@ -38,5 +38,4 @@ public void apply(ParseTree tree) {
             }
         }
     }
-
 }
diff --git a/...g/src/main/java/io/ecocode/ios/swift/checks/motionsensor/MotionSensorUpdateRateCheck.java b/...g/src/main/java/io/ecocode/ios/swift/checks/motionsensor/MotionSensorUpdateRateCheck.java
@@ -26,7 +26,7 @@
 import java.util.Arrays;
 import java.util.List;
 
-import static io.ecocode.ios.swift.checks.CheckHelper.isImportExisting;
+import static io.ecocode.ios.swift.checks.CheckHelper.*;
 
 @Rule(key="EC534")
 public class MotionSensorUpdateRateCheck extends SwiftRuleCheck {
@@ -43,12 +43,10 @@ public void apply(ParseTree tree) {
             importTree = (Swift5Parser.Import_declarationContext) tree;
             importExist = true;
         }
+        sensorRateUpdated = sensorRateUpdated || sensorRateUpdateExpressions.stream()
+                .anyMatch(exp -> isExpressionPresent(tree, exp));
 
-        if (!sensorRateUpdated && tree instanceof Swift5Parser.ExpressionContext) {
-            sensorRateUpdated = sensorRateUpdateExpressions.stream().anyMatch(exp -> tree.getText().contains(exp));
-        }
-
-        if (tree instanceof TerminalNodeImpl && tree.getText().equals("<EOF>")) {
+        if (isEndOfFile(tree)) {
             if (importExist && !sensorRateUpdated) {
                 this.recordIssue(importTree.getStart().getStartIndex(), DEFAULT_ISSUE_MESSAGE);
             }

diff --git a/swift-lang/src/main/java/io/ecocode/ios/swift/checks/sobriety/AnimationFreeCheck.java b/swift-lang/src/main/java/io/ecocode/ios/swift/checks/sobriety/AnimationFreeCheck.java
@@ -25,6 +25,8 @@
 import java.util.Arrays;
 import java.util.List;
 
+import static io.ecocode.ios.swift.checks.CheckHelper.isExpressionPresent;
+
 
 /**
  * Check the use of "UIScreen.main.brightness" and triggers when set.
@@ -43,16 +45,12 @@ public class AnimationFreeCheck extends SwiftRuleCheck {
 
     public void apply(ParseTree tree) {
         if (tree instanceof Swift5Parser.ExpressionContext) {
-            Swift5Parser.ExpressionContext id = (Swift5Parser.ExpressionContext) tree;
-            String expressionText = id.getText();
-
             boolean containsAnimationMethod = ANIMATION_METHODS.stream()
-                                                .anyMatch(expressionText::contains);
-
+                                                .anyMatch(animation -> isExpressionPresent(tree, animation));
              boolean containsUISwiftAnimationMethod = SWIFTUI_ANIMATION_METHODS.stream()
-                                                .anyMatch(expressionText::contains);             
-
+                                                .anyMatch(animation -> isExpressionPresent(tree, animation));
             if (containsAnimationMethod || containsUISwiftAnimationMethod) {
+                Swift5Parser.ExpressionContext id = (Swift5Parser.ExpressionContext) tree;
                 this.recordIssue(id.getStart().getStartIndex(), DEFAULT_ISSUE_MESSAGE);
             }
         }

diff --git a/swift-lang/src/main/java/io/ecocode/ios/swift/checks/sobriety/AudioRecorderLeakCheck.java b/swift-lang/src/main/java/io/ecocode/ios/swift/checks/sobriety/AudioRecorderLeakCheck.java
@@ -23,42 +23,45 @@
 import org.antlr.v4.runtime.tree.TerminalNodeImpl;
 import org.sonar.check.Rule;
 
+import static io.ecocode.ios.swift.checks.CheckHelper.*;
+
 @Rule(key = "EC515")
 public class AudioRecorderLeakCheck extends SwiftRuleCheck {
     private static final String DEFAULT_ISSUE_MESSAGE = "Any audio recording started should be stopped.";
     Swift5Parser.ExpressionContext id = null;
     private boolean audioRecorderStarted = false;
     private boolean audioRecorderStopped = false;
     private boolean importExist = false;
+    private boolean isAudioRecorderUsed = false;
 
 
     @Override
     public void apply(ParseTree tree) {
-        if (!importExist && tree instanceof Swift5Parser.ExpressionContext && tree.getText().contains("AVAudioRecorder")) {
-            importExist = true;
-        }
+        importExist = importExist || isImportExisting(tree, "AVFoundation");
+        isAudioRecorderUsed = isAudioRecorderUsed || (importExist && isExpressionPresent(tree, "AVAudioRecorder"));
 
-        if (importExist) {
+        if (isAudioRecorderUsed) {
             findStartedButNotStoppedAudioRecord(tree);
         }
     }
 
     private void findStartedButNotStoppedAudioRecord(ParseTree tree) {
-        if (tree instanceof Swift5Parser.ExpressionContext && tree.getText().contains("record()")) {
+        if (isFunctionCalled(tree, "record")) {
             id = (Swift5Parser.ExpressionContext) tree;
             audioRecorderStarted = true;
         }
 
-        if (tree instanceof Swift5Parser.ExpressionContext && (tree.getText().contains("stop()"))) {
+        if (isFunctionCalled(tree, "stop")) {
             audioRecorderStopped = true;
         }
 
-        if (tree instanceof TerminalNodeImpl && tree.getText().equals("<EOF>")) {
+        if (isEndOfFile(tree)) {
             if (audioRecorderStarted && !audioRecorderStopped) {
                 this.recordIssue(id.getStart().getStartIndex(), DEFAULT_ISSUE_MESSAGE);
             }
             audioRecorderStarted = false;
             audioRecorderStopped = false;
+            isAudioRecorderUsed = false;
             importExist = false;
         }
     }

diff --git a/swift-lang/src/main/java/io/ecocode/ios/swift/checks/sobriety/BrightnessOverrideCheck.java b/swift-lang/src/main/java/io/ecocode/ios/swift/checks/sobriety/BrightnessOverrideCheck.java
@@ -22,6 +22,8 @@
 import org.antlr.v4.runtime.tree.ParseTree;
 import org.sonar.check.Rule;
 
+import static io.ecocode.ios.swift.checks.CheckHelper.isExpressionPresent;
+
 /**
  * Check the use of "UIScreen.main.brightness" and triggers when set.
  */
@@ -31,11 +33,9 @@ public class BrightnessOverrideCheck extends SwiftRuleCheck {
     @Override
     public void apply(ParseTree tree) {
 
-    if (tree instanceof Swift5Parser.ExpressionContext) {
-        Swift5Parser.ExpressionContext id = (Swift5Parser.ExpressionContext) tree;
-        if (id.getText().contains("UIScreen.main.brightness")) {
+        if (isExpressionPresent(tree,"UIScreen.main.brightness")) {
+            Swift5Parser.ExpressionContext id = (Swift5Parser.ExpressionContext) tree;
             this.recordIssue(id.getStart().getStartIndex(), DEFAULT_ISSUE_MESSAGE);
         }
     }
 }
-}
diff --git a/...-lang/src/main/java/io/ecocode/ios/swift/checks/sobriety/FeedbackGeneratorUsageCheck.java b/...-lang/src/main/java/io/ecocode/ios/swift/checks/sobriety/FeedbackGeneratorUsageCheck.java
@@ -19,17 +19,19 @@
 
 import io.ecocode.ios.swift.SwiftRuleCheck;
 import io.ecocode.ios.swift.antlr.generated.Swift5Parser;
-import io.ecocode.ios.swift.checks.CheckHelper;
 import org.antlr.v4.runtime.tree.ParseTree;
-import org.antlr.v4.runtime.tree.TerminalNodeImpl;
 import org.sonar.check.Rule;
 
 import java.util.Objects;
 
+import static io.ecocode.ios.swift.checks.CheckHelper.*;
+
 @Rule(key = "EC528")
 public class FeedbackGeneratorUsageCheck extends SwiftRuleCheck {
     private static final String DEFAULT_ISSUE_MESSAGE = "Avoid using the device vibrator to use less energy.";
     public static final String UI_KIT = "UIKit";
+    public static final String IMPACT_OCCURRED = "impactOccurred";
+    public static final String UI_IMPACT_FEEDBACK_GENERATOR_CLASS = "UIImpactFeedbackGenerator";
     protected boolean isUIKitImported;
 
     protected Swift5Parser.ExpressionContext id;
@@ -40,16 +42,15 @@ public class FeedbackGeneratorUsageCheck extends SwiftRuleCheck {
     @Override
     public void apply(ParseTree tree) {
 
-        isUIKitImported = isUIKitImported || CheckHelper.isImportExisting(tree, UI_KIT);
+        isUIKitImported = isUIKitImported || isImportExisting(tree, UI_KIT);
 
         isFeedbackGeneratorInstantiated = isFeedbackGeneratorInstantiated ||
                 (isUIKitImported &&
-                        tree instanceof Swift5Parser.ExpressionContext &&
-                        (tree.getText().contains("UIImpactFeedbackGenerator")));
+                        isExpressionPresent(tree, UI_IMPACT_FEEDBACK_GENERATOR_CLASS));
 
         isImpactMethodCalled = isImpactMethodCalled ||
                 (isFeedbackGeneratorInstantiated &&
-                        (tree.getText().contains(".impactOccurred(")));
+                        isFunctionCalled(tree, IMPACT_OCCURRED));
 
         if (Objects.isNull(id) &&
                 tree instanceof Swift5Parser.ExpressionContext &&
@@ -58,7 +59,7 @@ public void apply(ParseTree tree) {
             id = (Swift5Parser.ExpressionContext) tree;
         }
 
-        if (tree instanceof TerminalNodeImpl && tree.getText().equals("<EOF>")) {
+        if (isEndOfFile(tree)) {
             if (isImpactMethodCalled) {
                 this.recordIssue(id.getStart().getStartIndex(), DEFAULT_ISSUE_MESSAGE);
             }

diff --git a/swift-lang/src/main/java/io/ecocode/ios/swift/checks/sobriety/LocationLeakCheck.java b/swift-lang/src/main/java/io/ecocode/ios/swift/checks/sobriety/LocationLeakCheck.java
@@ -20,30 +20,34 @@
 import io.ecocode.ios.swift.SwiftRuleCheck;
 import io.ecocode.ios.swift.antlr.generated.Swift5Parser;
 import org.antlr.v4.runtime.tree.ParseTree;
-import org.antlr.v4.runtime.tree.TerminalNodeImpl;
 import org.sonar.check.Rule;
 
+import static io.ecocode.ios.swift.checks.CheckHelper.isEndOfFile;
+import static io.ecocode.ios.swift.checks.CheckHelper.isFunctionCalled;
+
 
 @Rule(key = "EC513")
 public class LocationLeakCheck extends SwiftRuleCheck {
-    private static final String DEFAULT_ISSUE_MESSAGE = "calls must be carefully paired: CLLocationManager.startUpdatingLocation() and CLLocationManager.stopUpdatingLocation()";
+    public static final String START_UPDATING_LOCATION_METHOD = "startUpdatingLocation";
+    public static final String STOP_UPDATING_LOCATION_METHOD = "stopUpdatingLocation";
+    private static final String DEFAULT_ISSUE_MESSAGE = "calls must be carefully paired: CLLocationManager." + START_UPDATING_LOCATION_METHOD + "() and CLLocationManager." + STOP_UPDATING_LOCATION_METHOD + "()";
     protected boolean firstCallExist = false;
     protected boolean secondCallExist = false;
     protected Swift5Parser.ExpressionContext id;
 
     @Override
     public void apply(ParseTree tree) {
 
-        if (tree instanceof Swift5Parser.ExpressionContext && (tree.getText().contains(".startUpdatingLocation()"))) {
+        if (isFunctionCalled(tree, START_UPDATING_LOCATION_METHOD)) {
             firstCallExist = true;
             id = (Swift5Parser.ExpressionContext) tree;
         }
 
-        if (tree instanceof Swift5Parser.ExpressionContext && (tree.getText().contains(".stopUpdatingLocation()"))) {
+        if (isFunctionCalled(tree, STOP_UPDATING_LOCATION_METHOD)) {
             secondCallExist = true;
         }
 
-        if (tree instanceof TerminalNodeImpl && tree.getText().equals("<EOF>")) {
+        if (isEndOfFile(tree)) {
             if (firstCallExist && !secondCallExist) {
                 this.recordIssue(id.getStart().getStartIndex(), DEFAULT_ISSUE_MESSAGE);
             }

diff --git a/...lang/src/main/java/io/ecocode/ios/swift/checks/sobriety/LocationUpdatesDisabledCheck.java b/...lang/src/main/java/io/ecocode/ios/swift/checks/sobriety/LocationUpdatesDisabledCheck.java
@@ -22,20 +22,22 @@
 import org.antlr.v4.runtime.tree.ParseTree;
 import org.sonar.check.Rule;
 
+import static io.ecocode.ios.swift.checks.CheckHelper.isExpressionPresent;
+
 
 /**
  * Check the use of "CLLocationManager#pausesLocationUpdatesAutomatically" and triggers when set to false.
  */
 @Rule(key = "EC533")
 public class LocationUpdatesDisabledCheck extends SwiftRuleCheck {
     private static final String DEFAULT_ISSUE_MESSAGE = "Do not disable location updates pause, unless absolutely necessary";
+    public static final String LOCATION_UPDATES_DISABLE_EXPRESSION = ".pausesLocationUpdatesAutomatically=false";
+
     @Override
     public void apply(ParseTree tree) {
-        if (tree instanceof Swift5Parser.ExpressionContext) {
+        if(isExpressionPresent(tree, LOCATION_UPDATES_DISABLE_EXPRESSION)){
             Swift5Parser.ExpressionContext id = (Swift5Parser.ExpressionContext) tree;
-            if (id.getText().contains(".pausesLocationUpdatesAutomatically=false")) {
-                this.recordIssue(id.getStart().getStartIndex(), DEFAULT_ISSUE_MESSAGE);
-            }
+            this.recordIssue(id.getStart().getStartIndex(), DEFAULT_ISSUE_MESSAGE);
         }
     }
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -38,5 +38,4 @@ public void apply(ParseTree tree) { @@
                 }
             }
         }
     }