Merge pull request #3667 from coutvv/EO#3643-fix-codenarc

fix codenarc P2/P3 violations
objectionary · Dec 15, 2024 · f7f638b · f7f638b
2 parents efd9735 + 5298b96
commit f7f638b
Show file tree

Hide file tree

Showing 18 changed files with 279 additions and 223 deletions.
diff --git a/.codenarc b/.codenarc
@@ -220,7 +220,6 @@ ruleset {
     SpaceAfterSwitch
     SpaceAfterWhile
     SpaceAroundClosureArrow
-    SpaceAroundMapEntryColon
     SpaceAroundOperator
     SpaceBeforeClosingBrace
     SpaceBeforeOpeningBrace
@@ -430,7 +429,6 @@ ruleset {
     UnnecessaryPackageReference
     UnnecessaryParenthesesForMethodCallWithClosure
     UnnecessaryPublicModifier
-    UnnecessaryReturnKeyword
     UnnecessarySafeNavigationOperator
     UnnecessarySelfAssignment
     UnnecessarySemicolon

diff --git a/.github/workflows/codenarc.yml b/.github/workflows/codenarc.yml
@@ -44,7 +44,7 @@ jobs:
             org.codenarc.CodeNarc \
             -report=text:stdout \
             -maxPriority1Violations=0 \
-            -maxPriority2Violations=50 \
-            -maxPriority3Violations=200 \
+            -maxPriority2Violations=0 \
+            -maxPriority3Violations=0 \
             -failOnError=true \
             -rulesetfiles=file:.codenarc
diff --git a/eo-maven-plugin/src/it/custom_goals/verify.groovy b/eo-maven-plugin/src/it/custom_goals/verify.groovy
@@ -28,7 +28,7 @@
   'target/eo/2-optimize',
   'target/eo/3-shake',
   'target/eo/4-pull',
-].each { assert new File(basedir, it).exists() }
+].each { filePath -> assert new File(basedir, filePath).exists() }
 
 /**
  * Check that the pre and resolve goals had no effect.
@@ -39,6 +39,6 @@
   'target/eo/5-resolve',
   'target/eo/7-pre',
   'target/eo/8-transpile',
-].each { assert !new File(basedir, it).exists() }
+].each { filePath -> assert !new File(basedir, filePath).exists() }
 
 true
diff --git a/eo-maven-plugin/src/it/duplicate_classes/verify.groovy b/eo-maven-plugin/src/it/duplicate_classes/verify.groovy
@@ -1,4 +1,5 @@
 import java.nio.file.Files
+import java.nio.file.Path
 import java.util.stream.Collectors
 
 /**
@@ -25,16 +26,20 @@ import java.util.stream.Collectors
  * SOFTWARE.
  */
 
-def classes = basedir.toPath().resolve("target").resolve("classes")
-def testClasses = basedir.toPath().resolve("target").resolve("test-classes")
-def binaries = Files.walk(classes).filter(Files::isRegularFile)
-  .filter(file -> file.toString().endsWith(".class")).map {
-  return classes.relativize(it).toString()
-}.collect(Collectors.toSet())
-def disjoint = Files.walk(testClasses).filter(Files::isRegularFile)
-  .filter(file -> file.toString().endsWith(".class")).map {
-  return testClasses.relativize(it).toString()
-}.noneMatch { binaries.contains(it) }
-println "Compiled classes do not have duplicates: " + disjoint
+String target = 'target'
+String classExtension = '.class'
+
+Path classes = basedir.toPath().resolve(target).resolve('classes')
+Path testClasses = basedir.toPath().resolve(target).resolve('test-classes')
+Set<String> binaries = Files.walk(classes).filter(Files::isRegularFile)
+        .filter(file -> file.toString().endsWith(classExtension))
+        .map { path -> classes.relativize(path).toString() }
+        .collect(Collectors.toSet())
+boolean disjoint = Files.walk(testClasses).filter(Files::isRegularFile)
+        .filter(file -> file.toString().endsWith(classExtension))
+        .map { path -> testClasses.relativize(path).toString() }
+        .noneMatch { classPathName -> binaries.contains(classPathName) }
+
+log.info "Compiled classes do not have duplicates: $disjoint"
 assert disjoint
-return true
+return true
diff --git a/eo-maven-plugin/src/it/fibonacci/verify.groovy b/eo-maven-plugin/src/it/fibonacci/verify.groovy
@@ -28,45 +28,45 @@
  * @return Is the internet connection available
  */
 private static boolean online() {
-  boolean online = true
-  try {
-    final URL url = new URL("http://www.google.com")
-    final URLConnection conn = url.openConnection()
-    conn.connect()
-    conn.inputStream.close()
-  } catch (final IOException ignored) {
-    online = false
-  }
-  return online
+    boolean online = true
+    try {
+        final URL url = new URL('http://www.google.com')
+        final URLConnection conn = url.openConnection()
+        conn.connect()
+        conn.inputStream.close()
+    } catch (final IOException ignored) {
+        online = false
+    }
+    return online
 }
 
 [
-  'target/eo/foreign.csv',
-  'target/generated-sources/EOorg/EOeolang/EOexamples/EOapp.java',
-  'target/eo/1-parse/org/eolang/examples/app.xmir',
-  'target/eo/2-optimization-steps/org/eolang/examples/app/00-not-empty-atoms.xml',
-  'target/eo/2-optimize/org/eolang/examples/app.xmir',
-  'target/eo/6-pre/org/eolang/examples/app/01-classes.xml',
-  'target/eo/7-transpile/org/eolang/examples/app.xmir',
-  'target/eo/sodg/org/eolang/error.sodg',
-  'target/eo/sodg/org/eolang/error.sodg.xe',
-  'target/eo/sodg/org/eolang/error.sodg.graph.xml',
-  'target/eo/sodg/org/eolang/error.sodg.dot',
-].each { assert new File(basedir, it).exists() }
+        'target/eo/foreign.csv',
+        'target/generated-sources/EOorg/EOeolang/EOexamples/EOapp.java',
+        'target/eo/1-parse/org/eolang/examples/app.xmir',
+        'target/eo/2-optimization-steps/org/eolang/examples/app/00-not-empty-atoms.xml',
+        'target/eo/2-optimize/org/eolang/examples/app.xmir',
+        'target/eo/6-pre/org/eolang/examples/app/01-classes.xml',
+        'target/eo/7-transpile/org/eolang/examples/app.xmir',
+        'target/eo/sodg/org/eolang/error.sodg',
+        'target/eo/sodg/org/eolang/error.sodg.xe',
+        'target/eo/sodg/org/eolang/error.sodg.graph.xml',
+        'target/eo/sodg/org/eolang/error.sodg.dot',
+].each { path -> assert new File(basedir, path).exists() }
 
 [
-  'target/classes/EOorg/EOeolang/EOexamples/EOapp.class',
-  'target/eo/placed.json',
-  'target/eo/4-pull/org/eolang/tuple.eo',
-].each { assert new File(basedir, it).exists() || !online() }
+        'target/classes/EOorg/EOeolang/EOexamples/EOapp.class',
+        'target/eo/placed.json',
+        'target/eo/4-pull/org/eolang/tuple.eo',
+].each { path -> assert new File(basedir, path).exists() || !online() }
 
 String log = new File(basedir, 'build.log').text
 
 [
-  'org.eolang:eo-runtime:',
-  ' unpacked to ',
-  '6th Fibonacci number is 8',
-  'BUILD SUCCESS',
-].each { assert log.contains(it) || !online() }
+        'org.eolang:eo-runtime:',
+        ' unpacked to ',
+        '6th Fibonacci number is 8',
+        'BUILD SUCCESS',
+].each { expectedLog -> assert log.contains(expectedLog) || !online() }
 
 true
diff --git a/eo-maven-plugin/src/it/hash_package_layer/verify.groovy b/eo-maven-plugin/src/it/hash_package_layer/verify.groovy
@@ -22,8 +22,8 @@
  * SOFTWARE.
  */
 
-final String sources = "target/generated-sources"
-final String hash = "9c46a67"
+final String sources = 'target/generated-sources'
+final String hash = '9c46a67'
 final String[] files = [
   'EOorg/EOeolang/Heaps.java',
   'EOorg/EOeolang/package-info.java',
@@ -34,7 +34,7 @@ final String[] files = [
   'org/eolang/Versionized.java',
 ]
 
-files.each { assert new File(basedir, String.join(File.separator, sources, it)).exists() }
-files.each { assert new File(basedir, String.join(File.separator, sources, hash, it)).exists() }
+files.each { file -> assert new File(basedir, String.join(File.separator, sources, file)).exists() }
+files.each { file -> assert new File(basedir, String.join(File.separator, sources, hash, file)).exists() }
 
 true
diff --git a/eo-maven-plugin/src/it/rewritten_sources/verify.groovy b/eo-maven-plugin/src/it/rewritten_sources/verify.groovy
@@ -28,26 +28,26 @@
  * @return Is the internet connection available
  */
 private static boolean online() {
-  boolean online = true
-  try {
-    final URL url = new URL("http://www.google.com")
-    final URLConnection conn = url.openConnection()
-    conn.connect()
-    conn.inputStream.close()
-  } catch (final IOException ignored) {
-    online = false
-  }
-  return online
+    boolean online = true
+    try {
+        final URL url = new URL('http://www.google.com')
+        final URLConnection conn = url.openConnection()
+        conn.connect()
+        conn.inputStream.close()
+    } catch (final IOException ignored) {
+        online = false
+    }
+    return online
 }
 
 /**
  * Here we check if EOe.class exists and if it contains "EOorg/EOeolang/EOint" which means that is
  * wasn't overwritten.
  */
 if (online()) {
-  File euler =  new File(basedir, 'target/classes/EOorg/EOeolang/EOmath/EOe.class')
-  assert euler.exists()
-  assert euler.text.contains('EOorg/EOeolang/EOint')
+    File euler = new File(basedir, 'target/classes/EOorg/EOeolang/EOmath/EOe.class')
+    assert euler.exists()
+    assert euler.text.contains('EOorg/EOeolang/EOint')
 }
 
 true
diff --git a/eo-maven-plugin/src/test/groovy/check-parameters-names.groovy b/eo-maven-plugin/src/test/groovy/check-parameters-names.groovy
@@ -28,28 +28,32 @@
  */
 
 import groovy.xml.XmlSlurper
+import groovy.xml.slurpersupport.GPathResult
 
-plugin = basedir.toPath()
-    .resolve("target")
-    .resolve("classes")
-    .resolve("META-INF")
-    .resolve("maven")
-    .resolve("plugin.xml")
-content = new XmlSlurper().parseText(new File(plugin.toString()).text)
+import java.nio.file.Path
+
+Path plugin = basedir.toPath()
+    .resolve('target')
+    .resolve('classes')
+    .resolve('META-INF')
+    .resolve('maven')
+    .resolve('plugin.xml')
+GPathResult content = new XmlSlurper().parseText(new File(plugin.toString()).text)
 // For example, "${eo.foreignFormat}":
-pattern = "\\\$\\{eo\\.[a-z]+([A-Z][a-z]+)*}"
-failures = []
-toBeExcluded = ["help"]
-content.mojos.mojo.findAll {
-    !(it.goal.text() in toBeExcluded)
-}.configuration.each {
-    it.children().each {
-        final String text = it.text()
-        if (!("" == text || text.matches(pattern))) {
-            failures.add(text)
+String pattern = "\\\$\\{eo\\.[a-z]+([A-Z][a-z]+)*}"
+List<String> failures = []
+List<String> toBeExcluded = ['help']
+
+content.mojos.mojo.findAll { mojo -> !(mojo.goal.text() in toBeExcluded) }
+    .configuration
+    .each { mojoConf ->
+        mojoConf.children().each { child ->
+            final String text = child.text()
+            if (!(text == '' || text.matches(pattern))) {
+                failures.add(text)
+            }
         }
     }
-}
 if (!failures.empty) {
     fail(String.format(
         'Following parameters don\'t match pattern %s:%n %s',

diff --git a/eo-maven-plugin/src/test/groovy/verify.groovy b/eo-maven-plugin/src/test/groovy/verify.groovy
@@ -29,12 +29,12 @@ import java.nio.file.Path
  * To add new validation create new script in this folder and add it
  * to the list below.
  */
-Path folder = basedir.toPath().resolve("src").resolve("test").resolve("groovy")
-tests = [
+Path folder = basedir.toPath().resolve('src').resolve('test').resolve('groovy')
+List<String> tests = [
   'check-parameters-names.groovy'
 ]
-for (it in tests) {
-  def res = evaluate folder.resolve(it).toFile()
-  log.info String.format('Verified with %s - OK. Result: %s', it, res)
+for (test in tests) {
+    Object res = evaluate folder.resolve(test).toFile()
+    log.info "Verified with $test - OK. Result: $res"
 }
 true
diff --git a/eo-runtime/src/test/groovy/check-all-java-classes-compiled.groovy b/eo-runtime/src/test/groovy/check-all-java-classes-compiled.groovy
@@ -26,30 +26,30 @@ import java.nio.file.Files
 import java.nio.file.Path
 import java.util.stream.Collectors
 
+String org = 'org'
+String eolang = 'eolang'
+String javaExtension = '.java'
+String classExtension = '.class'
+
 Path binaries = basedir.toPath()
-    .resolve("target")
-    .resolve("classes")
-    .resolve("org")
-    .resolve("eolang");
+    .resolve('target')
+    .resolve('classes')
+    .resolve(org)
+    .resolve(eolang)
 Path classes = basedir.toPath()
-    .resolve("src")
-    .resolve("main")
-    .resolve("java")
-    .resolve("org")
-    .resolve("eolang")
+    .resolve('src')
+    .resolve('main')
+    .resolve('java')
+    .resolve(org)
+    .resolve(eolang)
 Set<String> expected = Files.walk(classes)
-    .filter(it -> {
-      it.toString().endsWith(".java")
-    })
+    .filter(path -> path.toString().endsWith(javaExtension))
     .map(Path::getFileName)
     .map(Path::toString)
-    .map(it -> {
-      return it.replace(".java", ".class")
-    }).collect(Collectors.toSet())
+    .map(pathName -> pathName.replace(javaExtension, classExtension))
+    .collect(Collectors.toSet())
 Set<String> actual = Files.walk(binaries)
-    .filter(it -> {
-      it.toString().endsWith(".class")
-    })
+    .filter(path -> path.toString().endsWith(classExtension))
     .map(Path::getFileName)
     .map(Path::toString)
     .collect(Collectors.toSet())

diff --git a/eo-runtime/src/test/groovy/check-folders-numbering.groovy b/eo-runtime/src/test/groovy/check-folders-numbering.groovy
@@ -22,16 +22,12 @@
  * SOFTWARE.
  */
 
+import java.nio.file.Path
 import java.util.stream.Collectors
 
-target = basedir.toPath().resolve('target/eo')
-List<File> directories = target.toFile().listFiles(new FileFilter() {
-    @Override
-    boolean accept(final File pathname) {
-        return pathname.isDirectory()
-    }
-})
-var allowed = [
+Path target = basedir.toPath().resolve('target/eo')
+List<File> directories = target.toFile().listFiles((FileFilter) { File file -> file.directory })
+List<String> allowed = [
     '1-parse',
     '2-optimize',
     '3-shake',
@@ -41,14 +37,14 @@ var allowed = [
     '7-pre',
     '8-transpile',
     'phi',
-    'unphi'
+    'unphi',
 ]
 List<File> allowedDirs = allowed.stream()
-    .map { target.resolve(it).toFile() }
+    .map { dirName -> target.resolve(dirName).toFile() }
     .collect(Collectors.toList())
 
 for (dir in directories) {
     if (!allowedDirs.contains(dir)) {
-        fail("The directory '${dir.name}' is not expected to be here");
+        fail("The directory '${dir.name}' is not expected to be here")
     }
 }
diff --git a/eo-runtime/src/test/groovy/check-runtime-deps.groovy b/eo-runtime/src/test/groovy/check-runtime-deps.groovy
@@ -28,12 +28,14 @@
  * jar (not fat-jar) and we expect that it won't require any outer dependencies
  */
 import groovy.xml.XmlSlurper
+import groovy.xml.slurpersupport.GPathResult
 
-def pom = new File('pom.xml').text
-def project = new XmlSlurper().parseText(pom)
+String pom = new File('pom.xml').text
+GPathResult project = new XmlSlurper().parseText(pom)
 
-project.dependencies.dependency.each {
-    if (it.scope.text() != 'test' && it.scope.text() != 'provided') {
-        fail("Dependency ${it.groupId.text()}.${it.artifactId.text()} must be in either 'test' or 'provided' scope")
+project.dependencies.dependency.each { dependency ->
+    if (dependency.scope.text() != 'test' && dependency.scope.text() != 'provided') {
+        fail("Dependency ${dependency.groupId.text()}.${dependency.artifactId.text()} " +
+                "must be in either 'test' or 'provided' scope")
     }
 }