Linting 2

StevenLooman · Jul 26, 2024 · 15ce6fa · 15ce6fa
1 parent 72109b8
commit 15ce6fa
Show file tree

Hide file tree

Showing 8 changed files with 68 additions and 149 deletions.
diff --git a/...ava/nl/ramsolutions/sw/magik/checks/checks/VariableDeclarationUsageDistanceCheckTest.java b/...ava/nl/ramsolutions/sw/magik/checks/checks/VariableDeclarationUsageDistanceCheckTest.java
@@ -5,87 +5,44 @@
 import java.util.List;
 import nl.ramsolutions.sw.magik.checks.MagikIssue;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 /** Test VariableDeclarationUsageDistanceCheck. */
 @SuppressWarnings("checkstyle:MagicNumber")
 class VariableDeclarationUsageDistanceCheckTest extends MagikCheckTestBase {
 
-  @Test
-  void testDistanceOk() {
-    final VariableDeclarationUsageDistanceCheck check = new VariableDeclarationUsageDistanceCheck();
-    check.maxDistance = 5;
-    final String code =
+  @ParameterizedTest
+  @ValueSource(
+      strings = {
         """
-        _method object.m
-            _local a << 1
-            do_something()
-            do_something()
-            write(a)
-        _endmethod""";
-    final List<MagikIssue> issues = this.runCheck(code, check);
-    assertThat(issues).isEmpty();
-  }
-
-  @Test
-  void testDistanceNotOk() {
-    final VariableDeclarationUsageDistanceCheck check = new VariableDeclarationUsageDistanceCheck();
-    check.maxDistance = 2;
-    final String code =
+          _method object.m
+              _local a << 1
+              do_something()
+              do_something()
+              write(a)
+          _endmethod""",
         """
-        _method object.m
-            _local a << 1
-            do_something()
-            do_something()
-            write(a)
-            write(a)
-        _endmethod""";
-    final List<MagikIssue> issues = this.runCheck(code, check);
-    assertThat(issues).hasSize(1);
-  }
-
-  @Test
-  void testUpperScopeDistanceOk() {
+          _method object.m
+              _local a << 1
+              do_something()
+              do_something()
+              _if _true
+              _then
+                  write(a)
+              _endif
+          _endmethod""",
+      })
+  void testValid(final String code) {
     final VariableDeclarationUsageDistanceCheck check = new VariableDeclarationUsageDistanceCheck();
     check.maxDistance = 5;
-    final String code =
-        """
-        _method object.m
-            _local a << 1
-            do_something()
-            do_something()
-            _if _true
-            _then
-                write(a)
-            _endif
-        _endmethod""";
     final List<MagikIssue> issues = this.runCheck(code, check);
     assertThat(issues).isEmpty();
   }
 
-  @Test
-  void testUpperScopeDistanceNotOk() {
-    final VariableDeclarationUsageDistanceCheck check = new VariableDeclarationUsageDistanceCheck();
-    check.maxDistance = 2;
-    final String code =
-        """
-        _method object.m
-            _local a << 1
-            do_something()
-            do_something()
-            _if _true
-            _then
-                write(a)
-            _endif
-        _endmethod""";
-    final List<MagikIssue> issues = this.runCheck(code, check);
-    assertThat(issues).hasSize(1);
-  }
-
-  @Test
-  void testVariableUsedInChildScope() {
-    final VariableDeclarationUsageDistanceCheck check = new VariableDeclarationUsageDistanceCheck();
-    check.maxDistance = 2;
-    final String code =
+  @ParameterizedTest
+  @ValueSource(
+      strings = {
         """
         _method object.m
             a << 1
@@ -98,16 +55,7 @@ void testVariableUsedInChildScope() {
                 a.method2()
             _endif
             >> a
-        _endmethod""";
-    final List<MagikIssue> issues = this.runCheck(code, check);
-    assertThat(issues).isEmpty();
-  }
-
-  @Test
-  void testDifferentScopesNotChecked() {
-    final VariableDeclarationUsageDistanceCheck check = new VariableDeclarationUsageDistanceCheck();
-    check.maxDistance = 2;
-    final String code =
+        _endmethod""",
         """
         _method object.m
             _if _true
@@ -120,76 +68,64 @@ void testDifferentScopesNotChecked() {
             _then
                 write(a)
             _endif
-        _endmethod""";
-    final List<MagikIssue> issues = this.runCheck(code, check);
-    assertThat(issues).isEmpty();
-  }
-
-  @Test
-  void testDistanceOkIndexerMethod() {
-    final VariableDeclarationUsageDistanceCheck check = new VariableDeclarationUsageDistanceCheck();
-    check.maxDistance = 2;
-    final String code =
+        _endmethod""",
         """
         _method object.m
             _local a << 1
             do_something()
             a[:abc] << :def
             a.do()
-        _endmethod""";
-    final List<MagikIssue> issues = this.runCheck(code, check);
-    assertThat(issues).isEmpty();
-  }
-
-  @Test
-  void testDistanceNotOkMethod() {
-    final VariableDeclarationUsageDistanceCheck check = new VariableDeclarationUsageDistanceCheck();
-    check.maxDistance = 2;
-    final String code =
+        _endmethod""",
         """
         _method object.m
-            _local a << 1
+            _constant a << 1
             do_something()
             do_something()
             do_something()
             a.method1()
-        _endmethod""";
-    final List<MagikIssue> issues = this.runCheck(code, check);
-    assertThat(issues).hasSize(1);
-  }
-
-  @Test
-  void testIgnoreConstants() {
+        _endmethod""",
+      })
+  void testValid2(final String code) {
     final VariableDeclarationUsageDistanceCheck check = new VariableDeclarationUsageDistanceCheck();
     check.maxDistance = 2;
-    check.ignoreConstants = true; // Defaults to try, but to be explicit.
-    final String code =
-        """
-        _method object.m
-            _constant a << 1
-            do_something()
-            do_something()
-            do_something()
-            a.method1()
-        _endmethod""";
+    check.ignoreConstants = true; // Defaults to true, but to be explicit.
     final List<MagikIssue> issues = this.runCheck(code, check);
     assertThat(issues).isEmpty();
   }
 
-  @Test
-  void testNotIgnoreConstants() {
-    final VariableDeclarationUsageDistanceCheck check = new VariableDeclarationUsageDistanceCheck();
-    check.maxDistance = 2;
-    check.ignoreConstants = false;
-    final String code =
+  @ParameterizedTest
+  @ValueSource(
+      strings = {
+        """
+          _method object.m
+              _local a << 1
+              do_something()
+              do_something()
+              write(a)
+              write(a)
+          _endmethod""",
+        """
+          _method object.m
+              _local a << 1
+              do_something()
+              do_something()
+              _if _true
+              _then
+                  write(a)
+              _endif
+          _endmethod""",
         """
         _method object.m
-            _constant a << 1
+            _local a << 1
             do_something()
             do_something()
             do_something()
             a.method1()
-        _endmethod""";
+        _endmethod""",
+      })
+  void testInvalid(final String code) {
+    final VariableDeclarationUsageDistanceCheck check = new VariableDeclarationUsageDistanceCheck();
+    check.maxDistance = 2;
     final List<MagikIssue> issues = this.runCheck(code, check);
     assertThat(issues).hasSize(1);
   }

diff --git a/...er/src/main/java/nl/ramsolutions/sw/magik/languageserver/MagikLanguageServerSettings.java b/...er/src/main/java/nl/ramsolutions/sw/magik/languageserver/MagikLanguageServerSettings.java
@@ -8,7 +8,6 @@
 /** Magik settings. */
 public final class MagikLanguageServerSettings {
 
-  private static final String SMALLWORLD_GIS = "magik.smallworldGis";
   private static final String PRODUCT_DIRS = "magik.productDirs";
   private static final String OVERRIDE_CONFIG_FILE = "magik.lint.overrideConfigFile";
   private static final String TYPE_DATABASE_PATHS = "magik.typing.typeDatabasePaths";

diff --git a/.../nl/ramsolutions/sw/magik/languageserver/semantictokens/ModuleDefSemanticTokenWalker.java b/.../nl/ramsolutions/sw/magik/languageserver/semantictokens/ModuleDefSemanticTokenWalker.java
@@ -11,7 +11,6 @@
 import java.util.stream.Collectors;
 import nl.ramsolutions.sw.definitions.analysis.ModuleDefAstWalker;
 import nl.ramsolutions.sw.definitions.api.SwModuleDefinitionKeyword;
-import nl.ramsolutions.sw.magik.ModuleDefFile;
 
 /** SwModuleDef semantic token walker. */
 public class ModuleDefSemanticTokenWalker extends ModuleDefAstWalker {
@@ -21,13 +20,8 @@ public class ModuleDefSemanticTokenWalker extends ModuleDefAstWalker {
           .map(SwModuleDefinitionKeyword::getValue)
           .collect(Collectors.toUnmodifiableSet());
 
-  private final ModuleDefFile moduleDefFile;
   private final List<SemanticToken> semanticTokens = new ArrayList<>();
 
-  public ModuleDefSemanticTokenWalker(final ModuleDefFile moduleDefFile) {
-    this.moduleDefFile = moduleDefFile;
-  }
-
   public List<SemanticToken> getSemanticTokens() {
     return Collections.unmodifiableList(this.semanticTokens);
   }

diff --git a/...nl/ramsolutions/sw/magik/languageserver/semantictokens/ProductDefSemanticTokenWalker.java b/...nl/ramsolutions/sw/magik/languageserver/semantictokens/ProductDefSemanticTokenWalker.java
@@ -11,7 +11,6 @@
 import java.util.stream.Collectors;
 import nl.ramsolutions.sw.definitions.analysis.ProductDefAstWalker;
 import nl.ramsolutions.sw.definitions.api.SwProductDefinitionKeyword;
-import nl.ramsolutions.sw.magik.ProductDefFile;
 
 /** SwProductDef semantic token walker. */
 public class ProductDefSemanticTokenWalker extends ProductDefAstWalker {
@@ -21,13 +20,8 @@ public class ProductDefSemanticTokenWalker extends ProductDefAstWalker {
           .map(SwProductDefinitionKeyword::getValue)
           .collect(Collectors.toUnmodifiableSet());
 
-  private final ProductDefFile productDefFile;
   private final List<SemanticToken> semanticTokens = new ArrayList<>();
 
-  public ProductDefSemanticTokenWalker(final ProductDefFile productDefFile) {
-    this.productDefFile = productDefFile;
-  }
-
   public List<SemanticToken> getSemanticTokens() {
     return Collections.unmodifiableList(this.semanticTokens);
   }

diff --git a/...in/java/nl/ramsolutions/sw/magik/languageserver/semantictokens/SemanticTokenProvider.java b/...in/java/nl/ramsolutions/sw/magik/languageserver/semantictokens/SemanticTokenProvider.java
@@ -76,7 +76,7 @@ public SemanticTokens provideSemanticTokensFull(final MagikTypedFile magikFile)
   public SemanticTokens provideSemanticTokensFull(final ModuleDefFile moduleDefFile) {
     LOGGER.debug("Providing semantic tokens full, file: {}", moduleDefFile);
 
-    final ModuleDefSemanticTokenWalker walker = new ModuleDefSemanticTokenWalker(moduleDefFile);
+    final ModuleDefSemanticTokenWalker walker = new ModuleDefSemanticTokenWalker();
     final AstNode topNode = moduleDefFile.getTopNode();
     walker.walkAst(topNode);
 
@@ -87,7 +87,7 @@ public SemanticTokens provideSemanticTokensFull(final ModuleDefFile moduleDefFil
   public SemanticTokens provideSemanticTokensFull(final ProductDefFile productDefFile) {
     LOGGER.debug("Providing semantic tokens full, file: {}", productDefFile);
 
-    final ProductDefSemanticTokenWalker walker = new ProductDefSemanticTokenWalker(productDefFile);
+    final ProductDefSemanticTokenWalker walker = new ProductDefSemanticTokenWalker();
     final AstNode topNode = productDefFile.getTopNode();
     walker.walkAst(topNode);
 

diff --git a/magik-lint/src/main/java/nl/ramsolutions/sw/magik/lint/MagikLint.java b/magik-lint/src/main/java/nl/ramsolutions/sw/magik/lint/MagikLint.java
@@ -43,7 +43,7 @@ public class MagikLint {
   /**
    * Constructor, parses command line and reads configuration.
    *
-   * @param configuration Configuration.
+   * @param properties Properites.
    * @param reporter Reporter.
    */
   public MagikLint(final MagikToolsProperties properties, final Reporter reporter) {
@@ -96,12 +96,12 @@ private List<MagikIssue> runCheckOnFile(final MagikFile magikFile, final MagikCh
   void showChecks(final Writer writer, final boolean showDisabled)
       throws ReflectiveOperationException, IOException {
     final Path overrideConfigPath = this.properties.getPropertyPath(MagikLint.KEY_OVERRIDE_CONFIG);
-    final MagikToolsProperties properties =
+    final MagikToolsProperties checksProperties =
         overrideConfigPath != null
             ? new MagikToolsProperties(overrideConfigPath)
             : MagikToolsProperties.DEFAULT_PROPERTIES;
     final MagikChecksConfiguration checksConfig =
-        new MagikChecksConfiguration(CheckList.getChecks(), properties);
+        new MagikChecksConfiguration(CheckList.getChecks(), checksProperties);
     final Iterable<MagikCheckHolder> holders = checksConfig.getAllChecks();
     for (final MagikCheckHolder holder : holders) {
       final MagikCheckMetadata metadata = holder.getMetadata();

diff --git a/magik-squid/src/main/java/nl/ramsolutions/sw/magik/analysis/scope/ScopeBuilderVisitor.java b/magik-squid/src/main/java/nl/ramsolutions/sw/magik/analysis/scope/ScopeBuilderVisitor.java
@@ -127,6 +127,7 @@ private void walkPreBodyMethodProcDefinition(final AstNode node, final AstNode p
             });
   }
 
+  @SuppressWarnings("java:S3776")
   @Override
   protected void walkPreVariableDefinitionStatement(final AstNode node) {
     final String type =
@@ -136,7 +137,7 @@ protected void walkPreVariableDefinitionStatement(final AstNode node) {
             .substring(1);
     final ScopeEntry.Type scopeEntryType = ScopeEntry.Type.valueOf(type);
 
-    Stream.concat(
+    Stream.of(
             // Definitions from VARIABLE_DEFINITION_MULTI.
             node.getChildren(MagikGrammar.VARIABLE_DEFINITION_MULTI).stream()
                 .map(
@@ -148,6 +149,7 @@ protected void walkPreVariableDefinitionStatement(final AstNode node) {
             // Defintions from VARIABLE_DEFINITION.
             node.getChildren(MagikGrammar.VARIABLE_DEFINITION).stream()
                 .map(varDefNode -> varDefNode.getFirstChild(MagikGrammar.IDENTIFIER)))
+        .flatMap(stream -> stream)
         // Work it!
         .filter(
             identifierNode -> {

diff --git a/...uid/src/test/java/nl/ramsolutions/sw/magik/analysis/definitions/DefinitionReaderTest.java b/...uid/src/test/java/nl/ramsolutions/sw/magik/analysis/definitions/DefinitionReaderTest.java
@@ -7,18 +7,12 @@
 import nl.ramsolutions.sw.magik.MagikFile;
 import nl.ramsolutions.sw.magik.analysis.typing.TypeString;
 import nl.ramsolutions.sw.magik.api.MagikGrammar;
-import nl.ramsolutions.sw.magik.parser.MagikParser;
 import org.junit.jupiter.api.Test;
 
 /** Test DefinitionReader. */
 @SuppressWarnings("checkstyle:MagicNumber")
 class DefinitionReaderTest {
 
-  private AstNode parseCode(String code) {
-    final MagikParser parser = new MagikParser();
-    return parser.parseSafe(code);
-  }
-
   private MagikFile createMagikFile(final String code) {
     return new MagikFile(MagikFile.DEFAULT_URI, code);
   }