Skip to content

Commit

Permalink
SONARPHP-1520 Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
GabinL21 committed Nov 8, 2024
1 parent 2a8624b commit 169a9eb
Show file tree
Hide file tree
Showing 24 changed files with 29 additions and 263 deletions.
53 changes: 0 additions & 53 deletions its/plugin/plugins/php-custom-rules-plugin/build.gradle.kts

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

13 changes: 4 additions & 9 deletions its/plugin/projects/custom_rules/src/simple.php
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
<?php

function f() {

$f1 = function () { // NOK - function expression
};

for (;;) { // NOK - for statement
}

}
some_function();
foo();
test();
buzz();
3 changes: 2 additions & 1 deletion its/plugin/tests/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ dependencies {
setIncludePatterns("Tests")
}

// Mandatory for the orchestrator in the "Tests" class, since it requires the custom rules plugin JAR
tasks.named("integrationTest") {
dependsOn(":its:plugin:plugins:php-custom-rules-plugin:build")
dependsOn(":php-custom-rules-plugin:shadowJar")
}

sonar {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
class CustomRulesTest {

@RegisterExtension
public static OrchestratorExtension orchestrator = Tests.ORCHESTRATOR;
public static final OrchestratorExtension orchestrator = Tests.ORCHESTRATOR;
private static final String PROJECT_KEY = "custom-rules";
private static final String PROJECT_NAME = "Custom Rules";
private static List<Issue> issues;
Expand All @@ -51,17 +51,17 @@ static void prepare() {

@Test
void baseTreeVisitorCheck() {
assertSingleIssue("php-custom-rules:visitor", 5, "Function expression.", "5min");
assertSingleIssue("custom:S1", 4, "Remove the usage of this forbidden function.", "5min");
}

@Test
void subscriptionBaseVisitorCheck() {
assertSingleIssue("php-custom-rules:subscription", 8, "For statement.", "10min");
assertSingleIssue("custom:S2", 6, "Remove the usage of this other forbidden function.", "10min");
}

private void assertSingleIssue(String ruleKey, int expectedLine, String expectedMessage, String expectedDebt) {
private static void assertSingleIssue(String ruleKey, int expectedLine, String expectedMessage, String expectedDebt) {
assertThat(Tests.issuesForRule(issues, ruleKey)).hasSize(1);
Issue issue = Tests.issuesForRule(issues, ruleKey).get(0);
var issue = Tests.issuesForRule(issues, ruleKey).get(0);
assertThat(issue.getLine()).isEqualTo(expectedLine);
assertThat(issue.getMessage()).isEqualTo(expectedMessage);
assertThat(issue.getDebt()).isEqualTo(expectedDebt);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class Tests {
.restoreProfileAtStartup(FileLocation.ofClasspath(RESOURCE_DIRECTORY + "profile.xml"))
.restoreProfileAtStartup(FileLocation.ofClasspath(RESOURCE_DIRECTORY + "no_rules.xml"))
// Custom rules plugin
.addPlugin(FileLocation.byWildcardFilename(new File("../plugins/php-custom-rules-plugin/build/libs"), "php-custom-rules-plugin-*-all.jar"))
.addPlugin(FileLocation.byWildcardFilename(new File("../../../php-custom-rules-plugin/build/libs"), "php-custom-rules-plugin-*-all.jar"))
.restoreProfileAtStartup(FileLocation.ofClasspath(RESOURCE_DIRECTORY + "profile-php-custom-rules.xml"))
.restoreProfileAtStartup(FileLocation.ofClasspath(RESOURCE_DIRECTORY + "nosonar.xml"))
.restoreProfileAtStartup(FileLocation.ofClasspath(RESOURCE_DIRECTORY + "sleep.xml"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,12 @@
<language>php</language>
<rules>
<rule>
<repositoryKey>php-custom-rules</repositoryKey>
<key>visitor</key>
<repositoryKey>custom</repositoryKey>
<key>S1</key>
</rule>
<rule>
<repositoryKey>php-custom-rules</repositoryKey>
<key>subscription</key>
</rule>
<rule>
<repositoryKey>deprecated-php-custom-rules</repositoryKey>
<key>visitor</key>
</rule>
<rule>
<repositoryKey>deprecated-php-custom-rules</repositoryKey>
<key>subscription</key>
<repositoryKey>custom</repositoryKey>
<key>S2</key>
</rule>
</rules>
</profile>
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ plugins {

dependencies {
compileOnly(libs.sonar.plugin.api)
compileOnly(project(":sonar-php-plugin"))
compileOnly(project(":php-frontend"))

testImplementation(libs.junit.jupiter)
testImplementation(libs.sonar.plugin.api.impl)
testImplementation(project(":sonar-php-plugin"))
testImplementation(project(":php-frontend"))
}

description = "PHP Custom Rules Example for SonarQube"
Expand All @@ -27,7 +27,7 @@ tasks.jar {
"Plugin-Display-Version" to version,
"Plugin-Homepage" to "https://sonarsource.atlassian.net/browse/SONARPHP",
"Plugin-IssueTrackerUrl" to "https://sonarsource.atlassian.net/browse/SONARPHP",
"Plugin-Key" to "php",
"Plugin-Key" to "custom",
"Plugin-License" to "GNU LGPL 3",
"Plugin-Name" to "PHP Custom Rules",
"Plugin-Organization" to "SonarSource",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public void define(Context context) {
// Optionally define remediation costs
Map<String, String> remediationCosts = new HashMap<>();
remediationCosts.put(ForbiddenFunctionUseCheck.KEY, "5min");
remediationCosts.put(OtherForbiddenFunctionUseCheck.KEY, "5min");
remediationCosts.put(OtherForbiddenFunctionUseCheck.KEY, "10min");
repository.rules().forEach(rule -> rule.setDebtRemediationFunction(
rule.debtRemediationFunctions().constantPerIssue(remediationCosts.get(rule.key()))));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@
)
public class ForbiddenFunctionUseCheck extends PHPVisitorCheck {

private static final Set<String> FORBIDDEN_FUNCTIONS = Set.of("foo", "bar");
public static final String KEY = "S1";
private static final Set<String> FORBIDDEN_FUNCTIONS = Set.of("foo", "bar");

/**
* Overriding method visiting the call expression to create an issue
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@
tags = {"convention"},
// Description can either be given in this annotation or through HTML name <ruleKey>.html located in package
// src/resources/org/sonar/l10n/php/rules/<repositoryKey>
description = "<p>The following functions should not be used:</p> <ul><li>foo</li> <li>bar</li></ul>")
description = "<p>The following functions should not be used:</p> <ul><li>fizz</li> <li>buzz</li></ul>")
public class OtherForbiddenFunctionUseCheck extends PHPSubscriptionCheck {

private static final Set<String> FORBIDDEN_FUNCTIONS = Set.of("foo", "bar");
public static final String KEY = "S2";
private static final Set<String> FORBIDDEN_FUNCTIONS = Set.of("fizz", "buzz");

@Override
public List<Kind> nodesToVisit() {
Expand All @@ -58,14 +58,14 @@ public List<Kind> nodesToVisit() {

/**
* Overriding method visiting the call expression to create an issue
* each time a call to "foo()" or "bar()" is done.
* each time a call to "fizz()" or "buzz()" is done.
*/
@Override
public void visitNode(Tree tree) {
ExpressionTree callee = ((FunctionCallTree) tree).callee();

if (callee.is(Kind.NAMESPACE_NAME) && FORBIDDEN_FUNCTIONS.contains(((NamespaceNameTree) callee).qualifiedName())) {
context().newIssue(this, callee, "Remove the usage of this forbidden function.");
context().newIssue(this, callee, "Remove the usage of this other forbidden function.");
}
}

Expand Down
Loading

0 comments on commit 169a9eb

Please sign in to comment.