Skip to content

Commit d347570

Browse files
jonas-wielage-sonarsourcejonas-wielage-sonarsource
authored
Revert "SONARPHP-1590 S1192 should not raise for HTML tags" (#1349)
1 parent e37fe4f commit d347570

File tree

5 files changed

+21
-262
lines changed

5 files changed

+21
-262
lines changed

‎its/ruling/src/integrationTest/resources/expected/PHPWord/php-S1192.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@
1313
"PHPWord:src/PhpWord/Writer/Word2007/Part/Styles.php": [
1414
101
1515
],
16+
"PHPWord:src/PhpWord/Writer/Word2007/Part/Theme.php": [
17+
224,
18+
227,
19+
228
20+
],
1621
"PHPWord:src/PhpWord/Writer/Word2007/Style/Font.php": [
1722
45
1823
],

‎php-checks/src/main/java/org/sonar/php/checks/StringLiteralDuplicatedCheck.java

Lines changed: 11 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.util.List;
2222
import java.util.Map;
2323
import java.util.regex.Pattern;
24+
import org.apache.commons.lang3.StringUtils;
2425
import org.sonar.check.Rule;
2526
import org.sonar.check.RuleProperty;
2627
import org.sonar.plugins.php.api.tree.CompilationUnitTree;
@@ -29,57 +30,20 @@
2930
import org.sonar.plugins.php.api.visitors.PHPVisitorCheck;
3031
import org.sonar.plugins.php.api.visitors.PreciseIssue;
3132

32-
import static org.sonar.php.checks.utils.RegexUtils.firstOf;
33-
import static org.sonar.php.checks.utils.RegexUtils.oneOrMore;
34-
import static org.sonar.php.checks.utils.RegexUtils.optional;
35-
import static org.sonar.php.checks.utils.RegexUtils.zeroOrMore;
36-
3733
@Rule(key = "S1192")
3834
public class StringLiteralDuplicatedCheck extends PHPVisitorCheck {
3935

4036
private static final String MESSAGE = "Define a constant instead of duplicating this literal \"%s\" %s times.";
4137
private static final String SECONDARY_MESSAGE = "Duplication.";
4238

43-
private static final String ONLY_ALPHANUMERIC_UNDERSCORES_HYPHENS_AND_PERIODS = "^[a-zA-Z_][.\\-\\w]+$";
44-
45-
// Single elements
46-
private static final String IDENTIFIER = "[a-zA-Z][a-zA-Z0-9\\-_:.]*+";
47-
private static final String OPT_SPACING = "\\s*+";
48-
private static final String OPT_TEXT_OUTSIDE_OF_TAGS = "[^<>]*+";
49-
private static final String DOUBLE_QUOTED_STRING = "\"(?:\\\\.|[^\"])*+\"";
50-
private static final String SINGLE_QUOTED_STRING = "'(?:\\\\.|[^'])*+'";
51-
private static final String NO_QUOTED_STRING = "[a-zA-Z0-9\\-_:./]++";
52-
53-
// Partial elements matching
54-
private static final String DOUBLE_QUOTED_STRING_PARTIAL_START = "\"(?:\\\\.|[^\"])*+";
55-
private static final String SINGLE_QUOTED_STRING_PARTIAL_START = "'(?:\\\\.|[^'])*+";
56-
private static final String NO_QUOTED_STRING_PARTIAL_START = "[a-zA-Z0-9\\-_:./]++";
57-
private static final String TAG_ATTRIBUTE_PARTIAL_START = OPT_SPACING
58-
+ optional("=", OPT_SPACING, optional(firstOf(DOUBLE_QUOTED_STRING_PARTIAL_START, SINGLE_QUOTED_STRING_PARTIAL_START, NO_QUOTED_STRING_PARTIAL_START)));
59-
60-
// Complex regexes
61-
private static final String TAG_ATTRIBUTE = IDENTIFIER + OPT_SPACING + optional("=", OPT_SPACING, firstOf(DOUBLE_QUOTED_STRING, SINGLE_QUOTED_STRING, NO_QUOTED_STRING));
62-
private static final String HTML_TAG_FULL = "</?" + OPT_SPACING + IDENTIFIER + OPT_SPACING + zeroOrMore(TAG_ATTRIBUTE, OPT_SPACING) + "/?+>";
63-
private static final String HTML_TAG_PARTIAL_START = OPT_SPACING + "</?+" + OPT_SPACING
64-
+ optional(IDENTIFIER, OPT_SPACING, zeroOrMore(TAG_ATTRIBUTE, OPT_SPACING), optional(TAG_ATTRIBUTE_PARTIAL_START));
65-
private static final String HTML_TAG_PARTIAL_END = "[\"']?+" + OPT_SPACING + zeroOrMore(TAG_ATTRIBUTE, OPT_SPACING) + "/?+>";
66-
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
67-
+ optional(HTML_TAG_PARTIAL_START);
68-
69-
private static final String FULL_ALLOWED_LITERALS_REGEX = firstOf(
70-
HTML_CONTENT,
71-
OPT_TEXT_OUTSIDE_OF_TAGS + HTML_TAG_PARTIAL_START,
72-
HTML_TAG_PARTIAL_END + OPT_TEXT_OUTSIDE_OF_TAGS,
73-
HTML_TAG_PARTIAL_END + OPT_TEXT_OUTSIDE_OF_TAGS + HTML_TAG_PARTIAL_START,
74-
ONLY_ALPHANUMERIC_UNDERSCORES_HYPHENS_AND_PERIODS);
75-
private static final Pattern ALLOWED_DUPLICATED_LITERALS = Pattern.compile(FULL_ALLOWED_LITERALS_REGEX);
76-
77-
public static final int THRESHOLD_DEFAULT = 3;
78-
public static final int MINIMAL_LITERAL_LENGTH_DEFAULT = 5;
39+
private static final Pattern ALLOWED_DUPLICATED_LITERALS = Pattern.compile("^[a-zA-Z_][.\\-\\w]+$");
7940

8041
private final Map<String, LiteralTree> firstOccurrenceTrees = new HashMap<>();
8142
private final Map<String, List<LiteralTree>> sameLiteralOccurrences = new HashMap<>();
8243

44+
public static final int THRESHOLD_DEFAULT = 3;
45+
public static final int MINIMAL_LITERAL_LENGTH_DEFAULT = 5;
46+
8347
@RuleProperty(
8448
key = "threshold",
8549
defaultValue = "" + THRESHOLD_DEFAULT)
@@ -102,10 +66,9 @@ public void visitCompilationUnit(CompilationUnitTree tree) {
10266

10367
private void finish() {
10468
for (Map.Entry<String, List<LiteralTree>> literalOccurrences : sameLiteralOccurrences.entrySet()) {
105-
String value = literalOccurrences.getKey();
10669
List<LiteralTree> occurrences = literalOccurrences.getValue();
10770

108-
if (occurrences.size() >= threshold && !ALLOWED_DUPLICATED_LITERALS.matcher(value).matches()) {
71+
if (occurrences.size() >= threshold) {
10972
String literal = literalOccurrences.getKey();
11073
String message = String.format(MESSAGE, literal, occurrences.size());
11174
LiteralTree firstOccurrenceTree = firstOccurrenceTrees.get(literal);
@@ -120,25 +83,22 @@ private void finish() {
12083
@Override
12184
public void visitLiteral(LiteralTree tree) {
12285
if (tree.is(Kind.REGULAR_STRING_LITERAL)) {
123-
String literal = tree.value().replace("\\'", "'").replace("\\\"", "\"");
124-
String value = removeQuotesAndQuotesEscaping(literal);
86+
String literal = tree.value();
87+
String value = StringUtils.substring(literal, 1, literal.length() - 1);
88+
89+
if (value.length() >= minimalLiteralLength && !ALLOWED_DUPLICATED_LITERALS.matcher(value).find()) {
12590

126-
if (value.length() >= minimalLiteralLength) {
12791
if (!sameLiteralOccurrences.containsKey(value)) {
12892
List<LiteralTree> occurrences = new ArrayList<>();
12993
occurrences.add(tree);
13094
sameLiteralOccurrences.put(value, occurrences);
13195
firstOccurrenceTrees.put(value, tree);
96+
13297
} else {
13398
sameLiteralOccurrences.get(value).add(tree);
13499
}
135100
}
136101
}
137102
}
138103

139-
private static String removeQuotesAndQuotesEscaping(String s) {
140-
var quote = s.charAt(0);
141-
return s.substring(1, s.length() - 1).replace("\\" + quote, String.valueOf(quote));
142-
}
143-
144104
}

‎php-checks/src/main/java/org/sonar/php/checks/utils/RegexUtils.java

Lines changed: 0 additions & 38 deletions
This file was deleted.

‎php-checks/src/test/java/org/sonar/php/checks/utils/RegexUtilsTest.java

Lines changed: 0 additions & 54 deletions
This file was deleted.

‎php-checks/src/test/resources/checks/StringLiteralDuplicatedCheck/default.php

Lines changed: 5 additions & 119 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,11 @@
3232
echo "$totototo";
3333

3434

35-
$a["name1_name-2."]; // OK - Only alphanumeric, -, _ and .
36-
$a["name1_name-2."];
37-
$a["name1_name-2."];
38-
$a["name1_name-2."];
39-
$a["name1_name-2."];
35+
$a["name1_name-2"]; // OK - Only alphanumeric, - and _
36+
$a["name1_name-2"];
37+
$a["name1_name-2"];
38+
$a["name1_name-2"];
39+
$a["name1_name-2"];
4040

4141
$a["name1/name2"]; // Noncompliant {{Define a constant instead of duplicating this literal "name1/name2" 5 times.}}
4242
$a["name1/name2"];
@@ -53,117 +53,3 @@
5353
$score = $request->getParam('_score'); // OK - Only alphanumeric, -, _ and .
5454
$score = $request->getParam('_score');
5555
$score = $request->getParam('_score');
56-
57-
$output = '';
58-
$output .= '<span class="payment">'.$total.'</span>';
59-
$output .= '<span class="payment-summary">'.$total_payment.'</span>';
60-
$output .= '<span class="badge normal">'.$inc_vat_text.'</span>';
61-
62-
$output .= '<div>'.$total.'</div>';
63-
$output .= '<div>'.$total_payment.'</div>';
64-
$output .= '<div>'.$inc_vat_text.'</div>';
65-
66-
$output = '<custom-tag_name:example.class>'.$var1.'</custom-tag_name:example.class>';
67-
$output = '<custom-tag_name:example.class>'.$var2.'</custom-tag_name:example.class>';
68-
$output = '<custom-tag_name:example.class>'.$var3.'</custom-tag_name:example.class>';
69-
70-
$output .= '<div class="beautiful">'.$total.'</div>';
71-
$output .= '<div class="beautiful">'.$total_payment.'</div>';
72-
$output .= '<div class="beautiful">'.$inc_vat_text.'</div>';
73-
74-
$output .= '<span class="payment">'.$total.'</span' + '>';
75-
$output .= '<span class="payment-summary">'.$total_payment.'</span' + '>';
76-
$output .= '<span class="badge normal">'.$inc_vat_text.'</span' + '>';
77-
78-
// HTML tag with attribute and escaping '
79-
$output .= '<div some-attribute-1="value1" some-attribute-2=\'value2\' some-attribute-3=value3 some-attribute-4>'.$total.'</div>';
80-
$output .= '<div some-attribute-1="value1" some-attribute-2=\'value2\' some-attribute-3=value3 some-attribute-4>'.$total.'</div>';
81-
$output .= '<div some-attribute-1="value1" some-attribute-2=\'value2\' some-attribute-3=value3 some-attribute-4>'.$total.'</div>';
82-
83-
// HTML tag with attribute and escaping "
84-
$output .= "<span class=\"payment\">".$total.'</span>';
85-
$output .= "<span class=\"payment\">".$total.'</span>';
86-
$output .= "<span class=\"payment\">".$total.'</span>';
87-
88-
// partial HTML tag start
89-
$output .= '<div some-attr='.$value.'>';
90-
$output .= '<div some-attr='.$value.'>';
91-
$output .= '<div some-attr='.$value.'>';
92-
93-
// partial HTML tag start with content before
94-
$test = 'When handling an <a href="http://';
95-
$test = 'When handling an <a href="http://';
96-
$test = 'When handling an <a href="http://';
97-
98-
// partial HTML tag end
99-
$output .= '<div><'.$tag_name.' with="value">';
100-
$output .= '<div><'.$tag_name.' with="value">';
101-
$output .= '<div><'.$tag_name.' with="value">';
102-
103-
// partial HTML tag end with content after
104-
$test = 'with="value"> some content after';
105-
$test = 'with="value"> some content after';
106-
$test = 'with="value"> some content after';
107-
108-
// multiple HTML tags
109-
$output .= '<div><p><span>'.$tag_name.'</div></p></span>';
110-
$output .= '<div><p><span>'.$tag_name.'</div></p></span>';
111-
$output .= '<div><p><span>'.$tag_name.'</div></p></span>';
112-
113-
// multiple HTML tags with text content outside of them
114-
$output .= '<div> some <p> content <span> here '.$tag_name.'</div> and </p> there </span>';
115-
$output .= '<div> some <p> content <span> here '.$tag_name.'</div> and </p> there </span>';
116-
$output .= '<div> some <p> content <span> here '.$tag_name.'</div> and </p> there </span>';
117-
118-
// multiple HTML tags with partial end or start tag
119-
$output .= ' left-from-previous="string"> here '.$tag_name.'</div><div';
120-
$output .= ' left-from-previous="string"> here '.$tag_name.'</div><div';
121-
$output .= ' left-from-previous="string"> here '.$tag_name.'</div><div';
122-
123-
// end and start tag in the same string literal
124-
$output .= 'end-of-tag="val"><div start-of-tag="val" '.$additionl_attribute;
125-
$output .= 'end-of-tag="val"><div start-of-tag="val" '.$additionl_attribute;
126-
$output .= 'end-of-tag="val"><div start-of-tag="val" '.$additionl_attribute;
127-
128-
// end and start tag with content between
129-
$test = 'end-of-tag="val"> some content <div start-of-tag="val" ';
130-
$test = 'end-of-tag="val"> some content <div start-of-tag="val" ';
131-
$test = 'end-of-tag="val"> some content <div start-of-tag="val" ';
132-
133-
// end and start tag with content and HTML tags between
134-
$test = 'end-of-tag="val"> some <i>extra</i> content <div start-of-tag="val" ';
135-
$test = 'end-of-tag="val"> some <i>extra</i> content <div start-of-tag="val" ';
136-
$test = 'end-of-tag="val"> some <i>extra</i> content <div start-of-tag="val" ';
137-
138-
// self closing tag
139-
$test = "<br/>";
140-
$test = "<br/>";
141-
$test = "<br/>";
142-
$test = "<br />";
143-
$test = "<br />";
144-
$test = "<br />";
145-
146-
// Examples that still raise an issue
147-
$test = "<!-- /wp:query -->"; // Noncompliant
148-
$test = "<!-- /wp:query -->";
149-
$test = "<!-- /wp:query -->";
150-
151-
$test = "/^<div>Content<\/div>$/"; // Noncompliant
152-
$test = "/^<div>Content<\/div>$/";
153-
$test = "/^<div>Content<\/div>$/";
154-
155-
$test = '<<B>%s</B>%s>'; // Noncompliant
156-
$test = '<<B>%s</B>%s>';
157-
$test = '<<B>%s</B>%s>';
158-
159-
$test = '<div><!--[if (gte mso 9)|(IE)]>'; // Noncompliant
160-
$test = '<div><!--[if (gte mso 9)|(IE)]>';
161-
$test = '<div><!--[if (gte mso 9)|(IE)]>';
162-
163-
$test = '$age < 5'; // Noncompliant
164-
$test = '$age < 5';
165-
$test = '$age < 5';
166-
167-
$test = '$name => "abc"'; // Noncompliant
168-
$test = '$name => "abc"';
169-
$test = '$name => "abc"';

0 commit comments

Comments
 (0)