diff --git a/.codenarc b/.codenarc index 8045a011be..6ac6de6664 100644 --- 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 index 60e9626e7b..e55e8ac6cb 100644 --- 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 index 9e9b4ff16b..9e9a8cb63c 100644 --- 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 index f0fc05e343..bab8e50a28 100644 --- 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 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 \ No newline at end of file +return true diff --git a/eo-maven-plugin/src/it/fibonacci/verify.groovy b/eo-maven-plugin/src/it/fibonacci/verify.groovy index d4bd9d5397..c4bc520a43 100644 --- 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 index 32c05e342b..1a4e887ece 100644 --- 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 index c081f09b15..b22c4901db 100644 --- a/eo-maven-plugin/src/it/rewritten_sources/verify.groovy +++ b/eo-maven-plugin/src/it/rewritten_sources/verify.groovy @@ -28,16 +28,16 @@ * @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 } /** @@ -45,9 +45,9 @@ private static boolean online() { * 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 index da356aac71..5fbf9bfab1 100644 --- 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 failures = [] +List 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 index b682074f8b..6ac111ff1d 100644 --- 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 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 index ce31836349..c32692a657 100644 --- 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 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 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 index d6b94d5dd2..8a4a1c3342 100644 --- 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 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 directories = target.toFile().listFiles((FileFilter) { File file -> file.directory }) +List allowed = [ '1-parse', '2-optimize', '3-shake', @@ -41,14 +37,14 @@ var allowed = [ '7-pre', '8-transpile', 'phi', - 'unphi' + 'unphi', ] List 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 index 7582bea862..1384e6a250 100644 --- 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") } } diff --git a/eo-runtime/src/test/groovy/check-target-files.groovy b/eo-runtime/src/test/groovy/check-target-files.groovy index afc3d290d2..8a113cd28c 100644 --- a/eo-runtime/src/test/groovy/check-target-files.groovy +++ b/eo-runtime/src/test/groovy/check-target-files.groovy @@ -24,7 +24,7 @@ import java.nio.file.Paths -var expected = [ +List expected = [ 'eo-foreign.csv', 'eo/1-parse/org/eolang/bytes.xmir', 'eo/1-parse/org/eolang/fs/dir.xmir', @@ -47,7 +47,7 @@ var expected = [ ] for (path in expected) { - var f = Paths.get('eo-runtime/target').resolve(path).toFile() + File f = Paths.get('eo-runtime/target').resolve(path).toFile() if (!f.exists()) { fail("The file '${f}' is not present") } diff --git a/eo-runtime/src/test/groovy/verify.groovy b/eo-runtime/src/test/groovy/verify.groovy index 057510c3c3..56cf46070c 100644 --- a/eo-runtime/src/test/groovy/verify.groovy +++ b/eo-runtime/src/test/groovy/verify.groovy @@ -30,7 +30,7 @@ import java.nio.file.Path * to the list below. */ Path folder = basedir.toPath().resolve('src/test/groovy') -var tests = [ +List tests = [ 'check-folders-numbering.groovy', 'check-all-java-classes-compiled.groovy', 'check-runtime-deps.groovy', @@ -39,3 +39,4 @@ var tests = [ for (it in tests) { evaluate folder.resolve(it).toFile() } + diff --git a/src/test/groovy/check-xsl-id.groovy b/src/test/groovy/check-xsl-id.groovy index 6a5c3a4646..15004fe4e0 100644 --- a/src/test/groovy/check-xsl-id.groovy +++ b/src/test/groovy/check-xsl-id.groovy @@ -29,19 +29,19 @@ import groovy.xml.XmlSlurper import groovy.io.FileType import groovy.io.FileVisitResult -project = new File('.') +File project = new File('.') project.traverse( type: FileType.FILES, - preDir: { - if (it.name == 'target') { + preDir: { file -> + if (file.name == 'target') { return FileVisitResult.SKIP_SUBTREE } }, - nameFilter: ~/.*\.xsl|xs3p.xsl/ + nameFilter: ~/.*\.xsl|xs3p.xsl/, ) { - it -> - String id = new XmlSlurper().parse(it).@id - assert id == it.name.minus('.xsl') + file -> + String id = new XmlSlurper().parse(file).@id + assert id == file.name - '.xsl' } true diff --git a/src/test/groovy/check-xsl-version.groovy b/src/test/groovy/check-xsl-version.groovy index 05c9c0c081..bf8c6a95a4 100644 --- a/src/test/groovy/check-xsl-version.groovy +++ b/src/test/groovy/check-xsl-version.groovy @@ -29,20 +29,20 @@ import groovy.xml.XmlSlurper import groovy.io.FileType import groovy.io.FileVisitResult -project = new File('.') +File project = new File('.') project.traverse( type: FileType.FILES, - preDir: { - if (it.name == 'target') { + preDir: { file -> + if (file.name == 'target') { return FileVisitResult.SKIP_SUBTREE } }, - nameFilter: ~/.*\.xsl/ + nameFilter: ~/.*\.xsl/, ) { - it -> - if (it.getName() == 'xs3p.xsl') { return } - String version = new XmlSlurper().parse(it).@version + file -> + if (file.name == 'xs3p.xsl') { return } + String version = new XmlSlurper().parse(file).@version assert version == '2.0' } diff --git a/src/test/groovy/verify.groovy b/src/test/groovy/verify.groovy index b2eb593618..b08475459a 100644 --- a/src/test/groovy/verify.groovy +++ b/src/test/groovy/verify.groovy @@ -27,11 +27,12 @@ * To add new validation create new script in this folder and add it * to the list below. */ + [ - 'src/test/groovy/check-xsl-id.groovy', - 'src/test/groovy/check-xsl-version.groovy', - 'src/test/groovy/xslqual.groovy' -].each { - evaluate(new File(it)) - println String.format('Verified with %s - OK', it) + 'src/test/groovy/check-xsl-id.groovy', + 'src/test/groovy/check-xsl-version.groovy', + 'src/test/groovy/xslqual.groovy', +].each { filename -> + evaluate(new File(filename)) + log.info "Verified with $filename - OK" } diff --git a/src/test/groovy/xslqual.groovy b/src/test/groovy/xslqual.groovy index da5ed507ce..49b6d4e23d 100644 --- a/src/test/groovy/xslqual.groovy +++ b/src/test/groovy/xslqual.groovy @@ -31,94 +31,143 @@ import groovy.io.FileType import groovy.io.FileVisitResult +import groovy.xml.XmlUtil -Map rules = [ - '//*[name()="xsl:for-each" or name()="xsl:if" or name()="xsl:when" or name()="xsl:otherwise"][(count(node()) = count(text())) and (normalize-space() = "")]': - 'Don\'t use empty content for instructions like \'xsl:for-each\' \'xsl:if\' \'xsl:when\' etc.', - '/xsl:stylesheet[@version = "2.0"]//@select[contains(., ":node-set")]': - 'Don\'t use node-set extension function if using XSLT 2.0', - '//xsl:function[not(some $x in //(@match | @select) satisfies contains($x, @name))]': - 'Stylesheet functions are unused', - '//xsl:template[@name and not(@match)][not(//xsl:call-template/@name = @name)]': - 'Named templates in stylesheet are unused', - '//xsl:variable[not(some $att in //@* satisfies contains($att, concat("$", @name)))]': - 'Variable is unused in the stylesheet', - '//*[name()="xsl:function" or name()="xsl:template"]/xsl:param[not(some $x in ..//(node() | @*) satisfies contains($x, concat("$", @name)))]': - 'Function or template parameter is unused in the function/template body', - '/xsl:stylesheet/xsl:output[@method = \'xml\'][starts-with(//xsl:template[.//html or .//HTML]/@match, "/")]': - 'Using the output method \'xml\' when generating HTML code', - '//@*[contains(., "name(") or contains(., "local-name(")]': - 'Using name() function when local-name() could be appropriate (and vice-versa)', - '//xsl:variable[(count(*) = 1) and (count(xsl:value-of) = 1)]': - 'Assign value to a variable using the "select" syntax if assigning a string value', - '//(@match | @select | @test)[starts-with(., "//")]': - 'Avoid using the operator // near the root of a large tree', - '//(@match | @select | @test)[not(starts-with(., "//")) and contains(., "//")]': - 'Avoid using the operator // in XPath expressions', - '//*[name()="xsl:template" or name()="xsl:function"][count(.//xsl:*) > 50]': - 'The function or template\'s size/complexity is high. There is need for refactoring the code.', - '//xsl:template[starts-with(@match, "")][(count(*) = count(xsl:variable)) and (normalize-space(string-join(text(), "")) = "")]': - 'The stylesheet is not generating any useful output. Please relook at the stylesheet logic.', - '//@*[name()="match" or name()="select"][contains(., "namespace::")][/xsl:stylesheet/@version = "2.0"]': - 'Using the deprecated namespace axis, when working in XSLT 2.0 mode', - '//@*[name()="match" or name()="select"][contains(., "child::") or contains(., "attribute::") or contains(., "parent::node()")]': - 'Using the lengthy axis specifiers like child::, attribute:: or parent::node()', - '//@disable-output-escaping[. = "yes"]': - 'Have set the disable-output-escaping attribute to \'yes\'. Please relook at the stylesheet logic.', - '//xsl:element[not(contains(@name, "$") or (contains(@name, "(") and contains(@name, ")")) or (contains(@name, "{") and contains(@name, "}")))]': - 'Creating an element node using the xsl:element instruction when could have been possible directly', - '//xsl:apply-templates[some $var in ancestor::xsl:template[1]//xsl:variable satisfies (($var << .) and starts-with(@select, $var/@name))]': - 'You might be confusing a variable reference with a node reference', - '//@*[(contains(., "true") and not(contains(., "true()"))) or (contains(., "false") and not(contains(., "false()")))]': - 'Incorrectly using the boolean constants as \'true\' or \'false\'', - '//*[name()="xsl:variable" or name()="xsl:template"][string-length(@name) = 1] | //xsl:function[string-length(substring-after(@name, ":")) = 1]': - 'Using a single character name for variable/function/template. Use meaningful names for these features.', - '//*[name()="xsl:variable" or name()="xsl:template"][(string-length(@name) > 1) and matches(@name, "[0-9].+")] | //xsl:function[(string-length(substring-after(@name, ":")) > 1) and matches(substring-after(@name, \':\'), \'[0-9].+\')]': - 'The variable/function/template name starts with a numeric character', -// '/xsl:stylesheet[count(//xsl:template[@match and not(@name)][count(*) < 3] ) >= 10]': -// 'Too many low granular templates in the stylesheet (10 or more)', -// '/xsl:stylesheet[count(//xsl:template | //xsl:function) = 1]': -// 'Using a single template/function in the stylesheet. You can modularize the code.', -// '/xsl:stylesheet[not(every $s in in-scope-prefixes(.)[not(. = "xml" or . = "")] satisfies exists(//(*[not(xsl:stylesheet)] | @*[not(parent::xsl:*)] | @select[parent::xsl:*] | @as | @name[parent::xsl:*])[starts-with(name(), concat($s, ":")) or starts-with(., concat($s, ":"))]))]': -// 'There are redundant namespace declarations in the xsl:stylesheet element', -// '/xsl:stylesheet[@version = "2.0"][not(some $x in .//@* satisfies contains($x, "xs:"))]': -// 'The stylesheet is not using any of the built-in Schema types (xs:string etc.), when working in XSLT 2.0 mode', -] +class MatchRule { -errors = 0 + String xpath + String warning + +} + +List rules = List.of(new MatchRule( + xpath: '//*[name()="xsl:for-each" or name()="xsl:if" or name()="xsl:when" or name()="xsl:otherwise"]' + + '[(count(node()) = count(text())) and (normalize-space() = "")]', + warning: 'Don\'t use empty content for instructions like \'xsl:for-each\' \'xsl:if\' \'xsl:when\' etc.' +), new MatchRule( + xpath: '/xsl:stylesheet[@version = "2.0"]//@select[contains(., ":node-set")]', + warning: 'Don\'t use node-set extension function if using XSLT 2.0' +), new MatchRule( + xpath: '//xsl:function[not(some $x in //(@match | @select) satisfies contains($x, @name))]', + warning: 'Stylesheet functions are unused' +), new MatchRule( + xpath: '//xsl:template[@name and not(@match)][not(//xsl:call-template/@name = @name)]', + warning: 'Named templates in stylesheet are unused' +), new MatchRule( + xpath: '//xsl:variable[not(some $att in //@* satisfies contains($att, concat("$", @name)))]', + warning: 'Variable is unused in the stylesheet' +), new MatchRule( + xpath: '//*[name()="xsl:function" or name()="xsl:template"]/xsl:param[not(some $x in ..//(node() ' + + '| @*) satisfies contains($x, concat("$", @name)))]', + warning: 'Function or template parameter is unused in the function/template body' +), new MatchRule( + xpath: '/xsl:stylesheet/xsl:output[@method = \'xml\']' + + '[starts-with(//xsl:template[.//html or .//HTML]/@match, "/")]', + warning: 'Using the output method \'xml\' when generating HTML code' +), new MatchRule( + xpath: '//@*[contains(., "name(") or contains(., "local-name(")]', + warning: 'Using name() function when local-name() could be appropriate (and vice-versa)' +), new MatchRule( + xpath: '//xsl:variable[(count(*) = 1) and (count(xsl:value-of) = 1)]', + warning: 'Assign value to a variable using the "select" syntax if assigning a string value' +), new MatchRule( + xpath: '//(@match | @select | @test)[starts-with(., "//")]', + warning: 'Avoid using the operator // near the root of a large tree' +), new MatchRule( + xpath: '//(@match | @select | @test)[not(starts-with(., "//")) and contains(., "//")]', + warning: 'Avoid using the operator // in XPath expressions' +), new MatchRule( + xpath: '//*[name()="xsl:template" or name()="xsl:function"][count(.//xsl:*) > 50]', + warning: 'The function or template\'s size/complexity is high. There is need for refactoring the code.' +), new MatchRule( + xpath: '//xsl:template[starts-with(@match, "")][(count(*) = count(xsl:variable)) ' + + 'and (normalize-space(string-join(text(), "")) = "")]', + warning: 'The stylesheet is not generating any useful output. Please relook at the stylesheet logic.' +), new MatchRule( + xpath: '//@*[name()="match" or name()="select"][contains(., "namespace::")][/xsl:stylesheet/@version = "2.0"]', + warning: 'Using the deprecated namespace axis, when working in XSLT 2.0 mode' +), new MatchRule( + xpath: '//@*[name()="match" or name()="select"][contains(., "child::") ' + + 'or contains(., "attribute::") or contains(., "parent::node()")]', + warning: 'Using the lengthy axis specifiers like child::, attribute:: or parent::node()' +), new MatchRule( + xpath: '//@disable-output-escaping[. = "yes"]', + warning: 'Have set the disable-output-escaping attribute to \'yes\'. Please relook at the stylesheet logic.' +), new MatchRule( + xpath: '//xsl:element[not(contains(@name, "$") or (contains(@name, "(") and contains(@name, ")")) ' + + 'or (contains(@name, "{") and contains(@name, "}")))]', + warning: 'Creating an element node using the xsl:element instruction when could have been possible directly' +), new MatchRule( + xpath: '//xsl:apply-templates[some $var in ancestor::xsl:template[1]//xsl:variable satisfies (($var << .) ' + + 'and starts-with(@select, $var/@name))]', + warning: 'You might be confusing a variable reference with a node reference' +), new MatchRule( + xpath: '//@*[(contains(., "true") and not(contains(., "true()"))) ' + + 'or (contains(., "false") and not(contains(., "false()")))]', + warning: 'Incorrectly using the boolean constants as \'true\' or \'false\'' +), new MatchRule( + xpath: '//*[name()="xsl:variable" or name()="xsl:template"][string-length(@name) = 1] ' + + '| //xsl:function[string-length(substring-after(@name, ":")) = 1]', + warning: 'Using a single character name for variable/function/template. ' + + 'Use meaningful names for these features.' +), new MatchRule( + xpath: '//*[name()="xsl:variable" or name()="xsl:template"][(string-length(@name) > 1) ' + + 'and matches(@name, "[0-9].+")] | //xsl:function[(string-length(substring-after(@name, ":")) > 1) ' + + 'and matches(substring-after(@name, \':\'), \'[0-9].+\')]', + warning: 'The variable/function/template name starts with a numeric character' +// commented early: +//), new MatchRule( +// xpath:'/xsl:stylesheet[count(//xsl:template[@match and not(@name)][count(*) < 3] ) >= 10]', +// warning:'Too many low granular templates in the stylesheet (10 or more)' +//), new MatchRule( +// xpath:'/xsl:stylesheet[count(//xsl:template | //xsl:function) = 1]', +// warning:'Using a single template/function in the stylesheet. You can modularize the code.' +//), new MatchRule( +// xpath:'/xsl:stylesheet[not(every $s in in-scope-prefixes(.)[not(. = "xml" or . = "")] ' + +// 'satisfies exists(//(*[not(xsl:stylesheet)] | @*[not(parent::xsl:*)] | @select[parent::xsl:*] | @as ' + +// '| @name[parent::xsl:*])[starts-with(name(), concat($s, ":")) or starts-with(., concat($s, ":"))]))]', +// warning:'There are redundant namespace declarations in the xsl:stylesheet element' +//), new MatchRule( +// xpath:'/xsl:stylesheet[@version = "2.0"][not(some $x in .//@* satisfies contains($x, "xs:"))]', +// warning:'The stylesheet is not using any of the built-in Schema types (xs:string etc.), ' + +// 'when working in XSLT 2.0 mode' +)) + +int errors = 0 rules.forEach { - xpath, warning -> - check = new com.jcabi.xml.XSLDocument( - """ + matchRule -> + String xpath = matchRule.xpath + String warning = matchRule.warning + com.jcabi.xml.XSLDocument check = new com.jcabi.xml.XSLDocument(""" - + at - """ - ) - println "${warning}:" - new File('.').traverse( - type: FileType.FILES, - preDir: { - if (it.name == 'target') { - return FileVisitResult.SKIP_SUBTREE - } - }, - nameFilter: ~/.*\.xsl/ - ) { - file -> - xsl = new com.jcabi.xml.XMLDocument(file) - ret = check.applyTo(xsl) - if (!ret.empty) { - println " ERROR: ${file} (${ret})" - ++errors - } - } + """) + println "${warning}:" + new File('.').traverse( + type: FileType.FILES, + preDir: { file -> + if (file.name == 'target') { + return FileVisitResult.SKIP_SUBTREE + } + }, + nameFilter: ~/.*\.xsl/, + ) { + file -> + com.jcabi.xml.XMLDocument xsl = new com.jcabi.xml.XMLDocument(file) + String ret = check.applyTo(xsl) + if (!ret.empty) { + println " ERROR: ${file} (${ret})" + ++errors + } + } } assert errors == 0 true +