diff --git a/its/ruling/src/integrationTest/resources/expected/PHPWord/php-S1192.json b/its/ruling/src/integrationTest/resources/expected/PHPWord/php-S1192.json index d38501ce92..c75921061b 100644 --- a/its/ruling/src/integrationTest/resources/expected/PHPWord/php-S1192.json +++ b/its/ruling/src/integrationTest/resources/expected/PHPWord/php-S1192.json @@ -13,6 +13,11 @@ "PHPWord:src/PhpWord/Writer/Word2007/Part/Styles.php": [ 101 ], +"PHPWord:src/PhpWord/Writer/Word2007/Part/Theme.php": [ +224, +227, +228 +], "PHPWord:src/PhpWord/Writer/Word2007/Style/Font.php": [ 45 ], diff --git a/php-checks/src/main/java/org/sonar/php/checks/StringLiteralDuplicatedCheck.java b/php-checks/src/main/java/org/sonar/php/checks/StringLiteralDuplicatedCheck.java index eb23975032..a223708881 100644 --- a/php-checks/src/main/java/org/sonar/php/checks/StringLiteralDuplicatedCheck.java +++ b/php-checks/src/main/java/org/sonar/php/checks/StringLiteralDuplicatedCheck.java @@ -21,6 +21,7 @@ import java.util.List; import java.util.Map; import java.util.regex.Pattern; +import org.apache.commons.lang3.StringUtils; import org.sonar.check.Rule; import org.sonar.check.RuleProperty; import org.sonar.plugins.php.api.tree.CompilationUnitTree; @@ -29,57 +30,20 @@ import org.sonar.plugins.php.api.visitors.PHPVisitorCheck; import org.sonar.plugins.php.api.visitors.PreciseIssue; -import static org.sonar.php.checks.utils.RegexUtils.firstOf; -import static org.sonar.php.checks.utils.RegexUtils.oneOrMore; -import static org.sonar.php.checks.utils.RegexUtils.optional; -import static org.sonar.php.checks.utils.RegexUtils.zeroOrMore; - @Rule(key = "S1192") public class StringLiteralDuplicatedCheck extends PHPVisitorCheck { private static final String MESSAGE = "Define a constant instead of duplicating this literal \"%s\" %s times."; private static final String SECONDARY_MESSAGE = "Duplication."; - private static final String ONLY_ALPHANUMERIC_UNDERSCORES_HYPHENS_AND_PERIODS = "^[a-zA-Z_][.\\-\\w]+$"; - - // Single elements - private static final String IDENTIFIER = "[a-zA-Z][a-zA-Z0-9\\-_:.]*+"; - private static final String OPT_SPACING = "\\s*+"; - private static final String OPT_TEXT_OUTSIDE_OF_TAGS = "[^<>]*+"; - private static final String DOUBLE_QUOTED_STRING = "\"(?:\\\\.|[^\"])*+\""; - private static final String SINGLE_QUOTED_STRING = "'(?:\\\\.|[^'])*+'"; - private static final String NO_QUOTED_STRING = "[a-zA-Z0-9\\-_:./]++"; - - // Partial elements matching - private static final String DOUBLE_QUOTED_STRING_PARTIAL_START = "\"(?:\\\\.|[^\"])*+"; - private static final String SINGLE_QUOTED_STRING_PARTIAL_START = "'(?:\\\\.|[^'])*+"; - private static final String NO_QUOTED_STRING_PARTIAL_START = "[a-zA-Z0-9\\-_:./]++"; - private static final String TAG_ATTRIBUTE_PARTIAL_START = OPT_SPACING - + optional("=", OPT_SPACING, optional(firstOf(DOUBLE_QUOTED_STRING_PARTIAL_START, SINGLE_QUOTED_STRING_PARTIAL_START, NO_QUOTED_STRING_PARTIAL_START))); - - // Complex regexes - private static final String TAG_ATTRIBUTE = IDENTIFIER + OPT_SPACING + optional("=", OPT_SPACING, firstOf(DOUBLE_QUOTED_STRING, SINGLE_QUOTED_STRING, NO_QUOTED_STRING)); - private static final String HTML_TAG_FULL = ""; - private static final String HTML_TAG_PARTIAL_START = OPT_SPACING + ""; - private static final String HTML_CONTENT = optional(HTML_TAG_PARTIAL_END) + oneOrMore(OPT_TEXT_OUTSIDE_OF_TAGS, HTML_TAG_FULL) + OPT_TEXT_OUTSIDE_OF_TAGS - + optional(HTML_TAG_PARTIAL_START); - - private static final String FULL_ALLOWED_LITERALS_REGEX = firstOf( - HTML_CONTENT, - OPT_TEXT_OUTSIDE_OF_TAGS + HTML_TAG_PARTIAL_START, - HTML_TAG_PARTIAL_END + OPT_TEXT_OUTSIDE_OF_TAGS, - HTML_TAG_PARTIAL_END + OPT_TEXT_OUTSIDE_OF_TAGS + HTML_TAG_PARTIAL_START, - ONLY_ALPHANUMERIC_UNDERSCORES_HYPHENS_AND_PERIODS); - private static final Pattern ALLOWED_DUPLICATED_LITERALS = Pattern.compile(FULL_ALLOWED_LITERALS_REGEX); - - public static final int THRESHOLD_DEFAULT = 3; - public static final int MINIMAL_LITERAL_LENGTH_DEFAULT = 5; + private static final Pattern ALLOWED_DUPLICATED_LITERALS = Pattern.compile("^[a-zA-Z_][.\\-\\w]+$"); private final Map firstOccurrenceTrees = new HashMap<>(); private final Map> sameLiteralOccurrences = new HashMap<>(); + public static final int THRESHOLD_DEFAULT = 3; + public static final int MINIMAL_LITERAL_LENGTH_DEFAULT = 5; + @RuleProperty( key = "threshold", defaultValue = "" + THRESHOLD_DEFAULT) @@ -102,10 +66,9 @@ public void visitCompilationUnit(CompilationUnitTree tree) { private void finish() { for (Map.Entry> literalOccurrences : sameLiteralOccurrences.entrySet()) { - String value = literalOccurrences.getKey(); List occurrences = literalOccurrences.getValue(); - if (occurrences.size() >= threshold && !ALLOWED_DUPLICATED_LITERALS.matcher(value).matches()) { + if (occurrences.size() >= threshold) { String literal = literalOccurrences.getKey(); String message = String.format(MESSAGE, literal, occurrences.size()); LiteralTree firstOccurrenceTree = firstOccurrenceTrees.get(literal); @@ -120,15 +83,17 @@ private void finish() { @Override public void visitLiteral(LiteralTree tree) { if (tree.is(Kind.REGULAR_STRING_LITERAL)) { - String literal = tree.value().replace("\\'", "'").replace("\\\"", "\""); - String value = removeQuotesAndQuotesEscaping(literal); + String literal = tree.value(); + String value = StringUtils.substring(literal, 1, literal.length() - 1); + + if (value.length() >= minimalLiteralLength && !ALLOWED_DUPLICATED_LITERALS.matcher(value).find()) { - if (value.length() >= minimalLiteralLength) { if (!sameLiteralOccurrences.containsKey(value)) { List occurrences = new ArrayList<>(); occurrences.add(tree); sameLiteralOccurrences.put(value, occurrences); firstOccurrenceTrees.put(value, tree); + } else { sameLiteralOccurrences.get(value).add(tree); } @@ -136,9 +101,4 @@ public void visitLiteral(LiteralTree tree) { } } - private static String removeQuotesAndQuotesEscaping(String s) { - var quote = s.charAt(0); - return s.substring(1, s.length() - 1).replace("\\" + quote, String.valueOf(quote)); - } - } diff --git a/php-checks/src/main/java/org/sonar/php/checks/utils/RegexUtils.java b/php-checks/src/main/java/org/sonar/php/checks/utils/RegexUtils.java deleted file mode 100644 index 689b044ec4..0000000000 --- a/php-checks/src/main/java/org/sonar/php/checks/utils/RegexUtils.java +++ /dev/null @@ -1,38 +0,0 @@ -/* - * SonarQube PHP Plugin - * Copyright (C) 2010-2024 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. - * See the Sonar Source-Available License for more details. - * - * You should have received a copy of the Sonar Source-Available License - * along with this program; if not, see https://sonarsource.com/license/ssal/ - */ -package org.sonar.php.checks.utils; - -public final class RegexUtils { - private RegexUtils() { - } - - public static String oneOrMore(String... s) { - return "(?:" + String.join("", s) + ")++"; - } - - public static String zeroOrMore(String... s) { - return "(?:" + String.join("", s) + ")*+"; - } - - public static String optional(String... s) { - return "(?:" + String.join("", s) + ")?+"; - } - - public static String firstOf(String... s) { - return "(?:(?:" + String.join(")|(?:", s) + "))"; - } -} diff --git a/php-checks/src/test/java/org/sonar/php/checks/utils/RegexUtilsTest.java b/php-checks/src/test/java/org/sonar/php/checks/utils/RegexUtilsTest.java deleted file mode 100644 index 4af364bca4..0000000000 --- a/php-checks/src/test/java/org/sonar/php/checks/utils/RegexUtilsTest.java +++ /dev/null @@ -1,54 +0,0 @@ -/* - * SonarQube PHP Plugin - * Copyright (C) 2010-2024 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. - * See the Sonar Source-Available License for more details. - * - * You should have received a copy of the Sonar Source-Available License - * along with this program; if not, see https://sonarsource.com/license/ssal/ - */ -package org.sonar.php.checks.utils; - -import org.junit.jupiter.api.Test; - -import static org.assertj.core.api.Assertions.assertThat; - -class RegexUtilsTest { - - @Test - void shouldCreateOneOrMoreRegexExpression() { - var regex = RegexUtils.oneOrMore("a", "b"); - assertThat(regex).isEqualTo("(?:ab)++"); - } - - @Test - void shouldCreateZeroOrMoreRegexExpression() { - var regex = RegexUtils.zeroOrMore("a", "b"); - assertThat(regex).isEqualTo("(?:ab)*+"); - } - - @Test - void shouldCreateOptionalRegexExpression() { - var regex = RegexUtils.optional("a", "b"); - assertThat(regex).isEqualTo("(?:ab)?+"); - } - - @Test - void shouldCreateFirstOfRegexExpression() { - var regex = RegexUtils.firstOf("a", "b"); - assertThat(regex).isEqualTo("(?:(?:a)|(?:b))"); - } - - @Test - void shouldCreateFirstOfRegexExpressionSingleValue() { - var regex = RegexUtils.firstOf("a"); - assertThat(regex).isEqualTo("(?:(?:a))"); - } -} diff --git a/php-checks/src/test/resources/checks/StringLiteralDuplicatedCheck/default.php b/php-checks/src/test/resources/checks/StringLiteralDuplicatedCheck/default.php index 003ed8a6e3..e297170394 100644 --- a/php-checks/src/test/resources/checks/StringLiteralDuplicatedCheck/default.php +++ b/php-checks/src/test/resources/checks/StringLiteralDuplicatedCheck/default.php @@ -32,11 +32,11 @@ echo "$totototo"; -$a["name1_name-2."]; // OK - Only alphanumeric, -, _ and . -$a["name1_name-2."]; -$a["name1_name-2."]; -$a["name1_name-2."]; -$a["name1_name-2."]; +$a["name1_name-2"]; // OK - Only alphanumeric, - and _ +$a["name1_name-2"]; +$a["name1_name-2"]; +$a["name1_name-2"]; +$a["name1_name-2"]; $a["name1/name2"]; // Noncompliant {{Define a constant instead of duplicating this literal "name1/name2" 5 times.}} $a["name1/name2"]; @@ -53,117 +53,3 @@ $score = $request->getParam('_score'); // OK - Only alphanumeric, -, _ and . $score = $request->getParam('_score'); $score = $request->getParam('_score'); - -$output = ''; -$output .= ''.$total.''; -$output .= ''.$total_payment.''; -$output .= ''.$inc_vat_text.''; - -$output .= '
'.$total.'
'; -$output .= '
'.$total_payment.'
'; -$output .= '
'.$inc_vat_text.'
'; - -$output = ''.$var1.''; -$output = ''.$var2.''; -$output = ''.$var3.''; - -$output .= '
'.$total.'
'; -$output .= '
'.$total_payment.'
'; -$output .= '
'.$inc_vat_text.'
'; - -$output .= ''.$total.''; -$output .= ''.$total_payment.''; -$output .= ''.$inc_vat_text.''; - -// HTML tag with attribute and escaping ' -$output .= '
'.$total.'
'; -$output .= '
'.$total.'
'; -$output .= '
'.$total.'
'; - -// HTML tag with attribute and escaping " -$output .= "".$total.''; -$output .= "".$total.''; -$output .= "".$total.''; - -// partial HTML tag start -$output .= '
'; -$output .= '
'; -$output .= '
'; - -// partial HTML tag start with content before -$test = 'When handling an '; -$output .= '
<'.$tag_name.' with="value">'; -$output .= '
<'.$tag_name.' with="value">'; - -// partial HTML tag end with content after -$test = 'with="value"> some content after'; -$test = 'with="value"> some content after'; -$test = 'with="value"> some content after'; - -// multiple HTML tags -$output .= '

'.$tag_name.'

'; -$output .= '

'.$tag_name.'

'; -$output .= '

'.$tag_name.'

'; - -// multiple HTML tags with text content outside of them -$output .= '
some

content here '.$tag_name.'

and

there '; -$output .= '
some

content here '.$tag_name.'

and

there '; -$output .= '
some

content here '.$tag_name.'

and

there '; - -// multiple HTML tags with partial end or start tag -$output .= ' left-from-previous="string"> here '.$tag_name.'