From cd7dbc6063ad097ee817cd3a85df8a19082c6f5f Mon Sep 17 00:00:00 2001 From: jycr Date: Fri, 14 Apr 2023 18:22:49 +0200 Subject: [PATCH] green-code-initiative/ecoCode#92 refactor(rule/php): moves PHP rules into `ecocode-rules-specifications` module --- ecocode-rules-specifications/pom.xml | 19 +++++ .../src/main/assembly/php.xml | 18 ++++ .../src/main/rules/EC22/EC22.json | 15 ++++ .../src/main/rules/EC22/php/EC22.asciidoc | 0 .../src/main/rules/EC3/php/EC3.asciidoc | 0 .../src/main/rules/EC34/EC34.json | 15 ++++ .../src/main/rules/EC34/php/EC34.asciidoc | 0 .../src/main/rules/EC4/php/EC4.asciidoc | 0 .../src/main/rules/EC66/EC66.json | 15 ++++ .../src/main/rules/EC66/php/EC66.asciidoc | 0 .../src/main/rules/EC67/php/EC67.asciidoc | 0 .../src/main/rules/EC69/php/EC69.asciidoc | 0 .../src/main/rules/EC72/php/EC72.asciidoc | 0 .../src/main/rules/EC74/php/EC74.asciidoc | 0 php-plugin/pom.xml | 28 ++++++- .../php/PhpRuleRepository.java | 73 ++++------------- .../php/PhpPluginTest.java | 16 ++-- .../php/PhpRuleRepositoryTest.java | 82 ++++++++++++++----- 18 files changed, 194 insertions(+), 87 deletions(-) create mode 100644 ecocode-rules-specifications/src/main/assembly/php.xml create mode 100644 ecocode-rules-specifications/src/main/rules/EC22/EC22.json rename php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC22.html => ecocode-rules-specifications/src/main/rules/EC22/php/EC22.asciidoc (100%) rename php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC3.html => ecocode-rules-specifications/src/main/rules/EC3/php/EC3.asciidoc (100%) create mode 100644 ecocode-rules-specifications/src/main/rules/EC34/EC34.json rename php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC34.html => ecocode-rules-specifications/src/main/rules/EC34/php/EC34.asciidoc (100%) rename php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC4.html => ecocode-rules-specifications/src/main/rules/EC4/php/EC4.asciidoc (100%) create mode 100644 ecocode-rules-specifications/src/main/rules/EC66/EC66.json rename php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC66.html => ecocode-rules-specifications/src/main/rules/EC66/php/EC66.asciidoc (100%) rename php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC67.html => ecocode-rules-specifications/src/main/rules/EC67/php/EC67.asciidoc (100%) rename php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC69.html => ecocode-rules-specifications/src/main/rules/EC69/php/EC69.asciidoc (100%) rename php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC72.html => ecocode-rules-specifications/src/main/rules/EC72/php/EC72.asciidoc (100%) rename php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC74.html => ecocode-rules-specifications/src/main/rules/EC74/php/EC74.asciidoc (100%) diff --git a/ecocode-rules-specifications/pom.xml b/ecocode-rules-specifications/pom.xml index 2d987929d..76686c851 100644 --- a/ecocode-rules-specifications/pom.xml +++ b/ecocode-rules-specifications/pom.xml @@ -104,6 +104,13 @@ + + + + + + + @@ -129,6 +136,18 @@ + + assembly-php + prepare-package + + single + + + + ${project.basedir}/src/main/assembly/php.xml + + + true diff --git a/ecocode-rules-specifications/src/main/assembly/php.xml b/ecocode-rules-specifications/src/main/assembly/php.xml new file mode 100644 index 000000000..83eb07521 --- /dev/null +++ b/ecocode-rules-specifications/src/main/assembly/php.xml @@ -0,0 +1,18 @@ + + php + + jar + + false + + + ${project.build.outputDirectory} + + io/ecocode/rules/php/*.* + + + + + diff --git a/ecocode-rules-specifications/src/main/rules/EC22/EC22.json b/ecocode-rules-specifications/src/main/rules/EC22/EC22.json new file mode 100644 index 000000000..9d3ed8c36 --- /dev/null +++ b/ecocode-rules-specifications/src/main/rules/EC22/EC22.json @@ -0,0 +1,15 @@ +{ + "title": "Use of methods for basic operations", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + "eco-design", + "performance", + "ecocode" + ], + "defaultSeverity": "Minor" +} diff --git a/php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC22.html b/ecocode-rules-specifications/src/main/rules/EC22/php/EC22.asciidoc similarity index 100% rename from php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC22.html rename to ecocode-rules-specifications/src/main/rules/EC22/php/EC22.asciidoc diff --git a/php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC3.html b/ecocode-rules-specifications/src/main/rules/EC3/php/EC3.asciidoc similarity index 100% rename from php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC3.html rename to ecocode-rules-specifications/src/main/rules/EC3/php/EC3.asciidoc diff --git a/ecocode-rules-specifications/src/main/rules/EC34/EC34.json b/ecocode-rules-specifications/src/main/rules/EC34/EC34.json new file mode 100644 index 000000000..4191cbb71 --- /dev/null +++ b/ecocode-rules-specifications/src/main/rules/EC34/EC34.json @@ -0,0 +1,15 @@ +{ + "title": "Avoid using try-catch-finally statement", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + "eco-design", + "performance", + "ecocode" + ], + "defaultSeverity": "Minor" +} diff --git a/php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC34.html b/ecocode-rules-specifications/src/main/rules/EC34/php/EC34.asciidoc similarity index 100% rename from php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC34.html rename to ecocode-rules-specifications/src/main/rules/EC34/php/EC34.asciidoc diff --git a/php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC4.html b/ecocode-rules-specifications/src/main/rules/EC4/php/EC4.asciidoc similarity index 100% rename from php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC4.html rename to ecocode-rules-specifications/src/main/rules/EC4/php/EC4.asciidoc diff --git a/ecocode-rules-specifications/src/main/rules/EC66/EC66.json b/ecocode-rules-specifications/src/main/rules/EC66/EC66.json new file mode 100644 index 000000000..6ed6137dc --- /dev/null +++ b/ecocode-rules-specifications/src/main/rules/EC66/EC66.json @@ -0,0 +1,15 @@ +{ + "title": "Avoid using double quote (\"), prefer using simple quote (')", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + "eco-design", + "performance", + "ecocode" + ], + "defaultSeverity": "Minor" +} diff --git a/php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC66.html b/ecocode-rules-specifications/src/main/rules/EC66/php/EC66.asciidoc similarity index 100% rename from php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC66.html rename to ecocode-rules-specifications/src/main/rules/EC66/php/EC66.asciidoc diff --git a/php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC67.html b/ecocode-rules-specifications/src/main/rules/EC67/php/EC67.asciidoc similarity index 100% rename from php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC67.html rename to ecocode-rules-specifications/src/main/rules/EC67/php/EC67.asciidoc diff --git a/php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC69.html b/ecocode-rules-specifications/src/main/rules/EC69/php/EC69.asciidoc similarity index 100% rename from php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC69.html rename to ecocode-rules-specifications/src/main/rules/EC69/php/EC69.asciidoc diff --git a/php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC72.html b/ecocode-rules-specifications/src/main/rules/EC72/php/EC72.asciidoc similarity index 100% rename from php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC72.html rename to ecocode-rules-specifications/src/main/rules/EC72/php/EC72.asciidoc diff --git a/php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC74.html b/ecocode-rules-specifications/src/main/rules/EC74/php/EC74.asciidoc similarity index 100% rename from php-plugin/src/main/resources/fr/greencodeinitiative/l10n/php/rules/custom/EC74.html rename to ecocode-rules-specifications/src/main/rules/EC74/php/EC74.asciidoc diff --git a/php-plugin/pom.xml b/php-plugin/pom.xml index ad70f265d..d9e1decad 100644 --- a/php-plugin/pom.xml +++ b/php-plugin/pom.xml @@ -16,16 +16,24 @@ https://github.com/green-code-initiative/ecoCode/tree/main/php-plugin + + ${project.groupId} + ecocode-rules-specifications + ${project.version} + php + org.sonarsource.php sonar-php-plugin sonar-plugin + provided org.sonarsource.sonarqube sonar-plugin-api + provided @@ -35,6 +43,19 @@ + + + org.sonarsource.java + java-checks-testkit + test + + + + org.junit.jupiter + junit-jupiter + test + + org.assertj assertj-core @@ -62,10 +83,13 @@ true ${sonarqube.version} ${java.version} - ${sonarqube.version} - ${java.version} + + + org.apache.maven.plugins + maven-shade-plugin + org.apache.maven.plugins maven-dependency-plugin diff --git a/php-plugin/src/main/java/fr/greencodeinitiative/php/PhpRuleRepository.java b/php-plugin/src/main/java/fr/greencodeinitiative/php/PhpRuleRepository.java index d0fd50e53..9c37e58e3 100644 --- a/php-plugin/src/main/java/fr/greencodeinitiative/php/PhpRuleRepository.java +++ b/php-plugin/src/main/java/fr/greencodeinitiative/php/PhpRuleRepository.java @@ -16,14 +16,7 @@ */ package fr.greencodeinitiative.php; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.InputStream; -import java.net.URL; -import java.nio.charset.StandardCharsets; -import java.util.HashMap; import java.util.List; -import java.util.Map; import fr.greencodeinitiative.php.checks.AvoidGettingSizeCollectionInLoopCheck; import fr.greencodeinitiative.php.checks.AvoidDoubleQuoteCheck; @@ -34,48 +27,29 @@ import fr.greencodeinitiative.php.checks.IncrementCheck; import fr.greencodeinitiative.php.checks.NoFunctionCallWhenDeclaringForLoop; import fr.greencodeinitiative.php.checks.UseOfMethodsForBasicOperations; -import org.sonar.api.rules.RuleType; +import org.sonar.api.SonarRuntime; import org.sonar.api.server.rule.RulesDefinition; -import org.sonar.api.server.rule.RulesDefinitionAnnotationLoader; import org.sonar.plugins.php.api.visitors.PHPCustomRuleRepository; +import org.sonarsource.analyzer.commons.RuleMetadataLoader; public class PhpRuleRepository implements RulesDefinition, PHPCustomRuleRepository { - public static final String LANGUAGE = "php"; - public static final String NAME = "ecoCode"; - public static final String RESOURCE_BASE_PATH = "/fr/greencodeinitiative/l10n/php/rules/custom/"; - public static final String REPOSITORY_KEY = "ecocode-php"; + private static final String LANGUAGE = "php"; + private static final String NAME = "ecoCode"; + private static final String RESOURCE_BASE_PATH = "io/ecocode/rules/php"; + private static final String REPOSITORY_KEY = "ecocode-php"; - @Override - public void define(Context context) { - NewRepository repository = context.createRepository(repositoryKey(), LANGUAGE).setName(NAME); - - new RulesDefinitionAnnotationLoader().load(repository, checkClasses().toArray(new Class[] {})); - - // technical debt - Map remediationCosts = new HashMap<>(); - remediationCosts.put(AvoidSQLRequestInLoopCheck.RULE_KEY, "10min"); - remediationCosts.put(AvoidFullSQLRequestCheck.RULE_KEY, "20min"); - repository.rules().forEach(rule -> { - rule.setType(RuleType.CODE_SMELL); - String debt = remediationCosts.get(rule.key()); - - // TODO DDC : create support to use org.apache.commons.lang.StringUtils -// if (StringUtils.isBlank(debt)) { - if (debt == null || debt.trim().equals("")) { - // default debt to 5min for issue correction - rule.setDebtRemediationFunction( - rule.debtRemediationFunctions().constantPerIssue("5min")); - } else { - rule.setDebtRemediationFunction( - rule.debtRemediationFunctions().constantPerIssue(debt)); - } - }); + private final SonarRuntime sonarRuntime; - // HTML description - repository.rules().forEach(rule -> - rule.setHtmlDescription(loadResource(RESOURCE_BASE_PATH + rule.key() + ".html"))); + public PhpRuleRepository(SonarRuntime sonarRuntime) { + this.sonarRuntime = sonarRuntime; + } + @Override + public void define(Context context) { + NewRepository repository = context.createRepository(REPOSITORY_KEY, LANGUAGE).setName(NAME); + RuleMetadataLoader ruleMetadataLoader = new RuleMetadataLoader(RESOURCE_BASE_PATH, sonarRuntime); + ruleMetadataLoader.addRulesByAnnotatedClass(repository, checkClasses()); repository.done(); } @@ -98,21 +72,4 @@ public List> checkClasses() { UseOfMethodsForBasicOperations.class ); } - - private String loadResource(String path) { - URL resource = getClass().getResource(path); - if (resource == null) { - throw new IllegalStateException("Resource not found: " + path); - } - ByteArrayOutputStream result = new ByteArrayOutputStream(); - try (InputStream in = resource.openStream()) { - byte[] buffer = new byte[1024]; - for (int len = in.read(buffer); len != -1; len = in.read(buffer)) { - result.write(buffer, 0, len); - } - return new String(result.toByteArray(), StandardCharsets.UTF_8); - } catch (IOException e) { - throw new IllegalStateException("Failed to read resource: " + path, e); - } - } } diff --git a/php-plugin/src/test/java/fr/greencodeinitiative/php/PhpPluginTest.java b/php-plugin/src/test/java/fr/greencodeinitiative/php/PhpPluginTest.java index 33736bc4c..9139c7c4e 100644 --- a/php-plugin/src/test/java/fr/greencodeinitiative/php/PhpPluginTest.java +++ b/php-plugin/src/test/java/fr/greencodeinitiative/php/PhpPluginTest.java @@ -16,20 +16,26 @@ */ package fr.greencodeinitiative.php; -import org.junit.Test; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.sonar.api.Plugin; import org.sonar.api.SonarRuntime; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; -public class PhpPluginTest { +class PhpPluginTest { + private Plugin.Context context; - @Test - public void test() { + @BeforeEach + void init() { SonarRuntime sonarRuntime = mock(SonarRuntime.class); - Plugin.Context context = new Plugin.Context(sonarRuntime); + context = new Plugin.Context(sonarRuntime); new PHPPlugin().define(context); + } + + @Test + void test() { assertThat(context.getExtensions()).hasSize(1); } diff --git a/php-plugin/src/test/java/fr/greencodeinitiative/php/PhpRuleRepositoryTest.java b/php-plugin/src/test/java/fr/greencodeinitiative/php/PhpRuleRepositoryTest.java index 70ef4cae8..709a7f889 100644 --- a/php-plugin/src/test/java/fr/greencodeinitiative/php/PhpRuleRepositoryTest.java +++ b/php-plugin/src/test/java/fr/greencodeinitiative/php/PhpRuleRepositoryTest.java @@ -16,42 +16,80 @@ */ package fr.greencodeinitiative.php; -import static org.assertj.core.api.Assertions.assertThat; import org.assertj.core.api.SoftAssertions; -import org.junit.Before; -import org.junit.Test; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.sonar.api.SonarRuntime; import org.sonar.api.server.rule.RulesDefinition; +import org.sonar.api.utils.Version; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; + +class PhpRuleRepositoryTest { -public class PhpRuleRepositoryTest { + private RulesDefinition.Repository repository; - private PhpRuleRepository phpRuleRepository; - private RulesDefinition.Context context; + @BeforeEach + void init() { + // TODO: Remove this check after Git repo split + if (PhpRuleRepository.class.getResource("/io/ecocode/rules/php/EC4.json") == null) { + String message = "'ecocode-rules-specification' resources corrupted. Please check build of 'ecocode-rules-specification' module"; + if (System.getProperties().keySet().stream().anyMatch(k -> k.toString().startsWith("idea."))) { + message += "\n\nOn 'IntelliJ IDEA':" + + "\n1. go to settings :" + + "\n > Build, Execution, Deployment > Build Tools > Maven > Runner" + + "\n2. check option:" + + "\n > Delegate IDE build/run actions to Maven" + + "\n3. Click on menu: " + + "\n > Build > Build Project" + ; + } + fail(message); + } - @Before - public void init() { - phpRuleRepository = new PhpRuleRepository(); - context = new RulesDefinition.Context(); - phpRuleRepository.define(context); + final SonarRuntime sonarRuntime = mock(SonarRuntime.class); + doReturn(Version.create(0, 0)).when(sonarRuntime).getApiVersion(); + PhpRuleRepository rulesDefinition = new PhpRuleRepository(sonarRuntime); + RulesDefinition.Context context = new RulesDefinition.Context(); + rulesDefinition.define(context); + repository = context.repository(rulesDefinition.repositoryKey()); + } + + @Test + @DisplayName("Test repository metadata") + void testMetadata() { + assertThat(repository.name()).isEqualTo("ecoCode"); + assertThat(repository.language()).isEqualTo("php"); + assertThat(repository.key()).isEqualTo("ecocode-php"); } @Test - public void test() { - assertThat(phpRuleRepository.repositoryKey()).isEqualTo(PhpRuleRepository.REPOSITORY_KEY); - assertThat(context.repositories()).hasSize(1).extracting("key").containsExactly(phpRuleRepository.repositoryKey()); - assertThat(context.repositories().get(0).rules()).hasSize(9); - assertThat(phpRuleRepository.checkClasses()).hasSize(9); + void testRegistredRules() { + assertThat(repository.rules()).hasSize(9); } - /** - * Check all rule keys must be prefixed by 'EC' - */ - @Test() - public void testRuleKeyPrefix() { - RulesDefinition.Repository repository = context.repository(PhpRuleRepository.REPOSITORY_KEY); + @Test + @DisplayName("All rule keys must be prefixed by 'EC'") + void testRuleKeyPrefix() { SoftAssertions assertions = new SoftAssertions(); repository.rules().forEach( rule -> assertions.assertThat(rule.key()).startsWith("EC") ); assertions.assertAll(); } + + @Test + void testAllRuleParametersHaveDescription() { + SoftAssertions assertions = new SoftAssertions(); + repository.rules().stream() + .flatMap(rule -> rule.params().stream()) + .forEach(param -> assertions.assertThat(param.description()) + .as("description for " + param.key()) + .isNotEmpty()); + assertions.assertAll(); + } }