SONARPHP-1626 S1541 should detect elseif clauses (#1374)

nils-werner-sonarsource · web-flow · commit 801bf3cc849b · 2025-02-12T09:00:53.000+01:00
diff --git a/its/plugin/src/integrationTest/java/com/sonar/it/php/PHPIntegrationTest.java b/its/plugin/src/integrationTest/java/com/sonar/it/php/PHPIntegrationTest.java
@@ -69,7 +69,7 @@ void projectMetric() {
       softly.assertThat(getProjectMeasureAsDouble("public_api")).isNull();
 
       // Complexity
-      softly.assertThat(getProjectMeasureAsDouble("complexity")).isEqualTo(4341d);
+      softly.assertThat(getProjectMeasureAsDouble("complexity")).isEqualTo(4440d);
       softly.assertThat(getProjectMeasureAsDouble("cognitive_complexity")).isEqualTo(3203d);
     });
   }
@@ -96,7 +96,7 @@ void fileMetrics() {
       softly.assertThat(getFileMeasureAsDouble("public_api")).isNull();
 
       // Complexity
-      softly.assertThat(getFileMeasureAsDouble("complexity")).isEqualTo(14d);
+      softly.assertThat(getFileMeasureAsDouble("complexity")).isEqualTo(15d);
       softly.assertThat(getFileMeasureAsDouble("cognitive_complexity")).isEqualTo(14d);
     });
   }
diff --git a/its/ruling/src/integrationTest/java/org/sonar/php/it/PhpGeneralRulingTest.java b/its/ruling/src/integrationTest/java/org/sonar/php/it/PhpGeneralRulingTest.java
@@ -66,7 +66,7 @@ void testFlysystem() throws Exception {
   void testMonica() throws Exception {
     // To avoid error: File tests/Unit/Traits/SearchableTest.php can't be indexed twice...
     // the tests directory needs to be excluded
-    testProject("monica", "sonar.exclusions", "**/tests/**");
+    testProject("monica", "sonar.exclusions", "tests/**");
   }
 
   @Test
diff --git a/its/ruling/src/integrationTest/resources/expected/PHPMailer/php-S1541.json b/its/ruling/src/integrationTest/resources/expected/PHPMailer/php-S1541.json
@@ -3,6 +3,7 @@
 173
 ],
 "PHPMailer:src/PHPMailer.php": [
+1082,
 1222,
 1540,
 1724,
@@ -24,5 +25,8 @@
 512,
 730,
 1253
+],
+"PHPMailer:test/TestCase.php": [
+141
 ]
 }
diff --git a/its/ruling/src/integrationTest/resources/expected/PHPWord/php-S1541.json b/its/ruling/src/integrationTest/resources/expected/PHPWord/php-S1541.json
@@ -2,6 +2,9 @@
 "PHPWord:src/PhpWord/Element/Image.php": [
 530
 ],
+"PHPWord:src/PhpWord/Escaper/Rtf.php": [
+52
+],
 "PHPWord:src/PhpWord/Reader/MsDoc.php": [
 1536,
 1608,
@@ -13,7 +16,8 @@
 "PHPWord:src/PhpWord/Reader/Word2007/AbstractPart.php": [
 191,
 331,
-493
+493,
+596
 ],
 "PHPWord:src/PhpWord/Reader/Word2007/Numbering.php": [
 34
@@ -49,7 +53,9 @@
 4309,
 4458,
 4784,
-5191
+5114,
+5191,
+5269
 ],
 "PHPWord:src/PhpWord/TemplateProcessor.php": [
 530,
diff --git a/its/ruling/src/integrationTest/resources/expected/PhpSpreadsheet/php-S1311.json b/its/ruling/src/integrationTest/resources/expected/PhpSpreadsheet/php-S1311.json
@@ -23,6 +23,9 @@
 "PhpSpreadsheet:src/PhpSpreadsheet/Writer/Html.php": [
 33
 ],
+"PhpSpreadsheet:src/PhpSpreadsheet/Writer/Xls/Parser.php": [
+34
+],
 "PhpSpreadsheet:src/PhpSpreadsheet/Writer/Xls/Worksheet.php": [
 56
 ],
diff --git a/its/ruling/src/integrationTest/resources/expected/PhpSpreadsheet/php-S1541.json b/its/ruling/src/integrationTest/resources/expected/PhpSpreadsheet/php-S1541.json
@@ -8,15 +8,20 @@
 3634,
 3785,
 3828,
+3870,
 4039,
 4551,
+5175,
 5269,
 5422,
 5486
 ],
 "PhpSpreadsheet:src/PhpSpreadsheet/Calculation/DateTimeExcel/DateValue.php": [
 43
 ],
+"PhpSpreadsheet:src/PhpSpreadsheet/Calculation/DateTimeExcel/TimeValue.php": [
+39
+],
 "PhpSpreadsheet:src/PhpSpreadsheet/Calculation/Financial/CashFlow/Variable/NonPeriodic.php": [
 33
 ],
@@ -29,6 +34,12 @@
 "PhpSpreadsheet:src/PhpSpreadsheet/Calculation/Functions.php": [
 146
 ],
+"PhpSpreadsheet:src/PhpSpreadsheet/Calculation/Information/Value.php": [
+281
+],
+"PhpSpreadsheet:src/PhpSpreadsheet/Calculation/Logical/Operations.php": [
+133
+],
 "PhpSpreadsheet:src/PhpSpreadsheet/Calculation/LookupRef/Address.php": [
 50
 ],
@@ -49,6 +60,9 @@
 "PhpSpreadsheet:src/PhpSpreadsheet/Calculation/Statistical/AggregateBase.php": [
 39
 ],
+"PhpSpreadsheet:src/PhpSpreadsheet/Calculation/Statistical/Deviations.php": [
+105
+],
 "PhpSpreadsheet:src/PhpSpreadsheet/Calculation/Statistical/Distributions/Beta.php": [
 39,
 92
@@ -62,6 +76,10 @@
 "PhpSpreadsheet:src/PhpSpreadsheet/Calculation/Statistical/Distributions/HyperGeometric.php": [
 32
 ],
+"PhpSpreadsheet:src/PhpSpreadsheet/Calculation/Statistical/Variances.php": [
+64,
+153
+],
 "PhpSpreadsheet:src/PhpSpreadsheet/Calculation/TextData/Format.php": [
 281
 ],
@@ -77,7 +95,8 @@
 616
 ],
 "PhpSpreadsheet:src/PhpSpreadsheet/Cell/DataValidator.php": [
-19
+19,
+55
 ],
 "PhpSpreadsheet:src/PhpSpreadsheet/Cell/DefaultValueBinder.php": [
 46
@@ -88,6 +107,9 @@
 "PhpSpreadsheet:src/PhpSpreadsheet/CellReferenceHelper.php": [
 44
 ],
+"PhpSpreadsheet:src/PhpSpreadsheet/Chart/ChartColor.php": [
+84
+],
 "PhpSpreadsheet:src/PhpSpreadsheet/Chart/DataSeriesValues.php": [
 429
 ],
@@ -117,6 +139,7 @@
 849
 ],
 "PhpSpreadsheet:src/PhpSpreadsheet/Reader/Ods.php": [
+46,
 140,
 246
 ],
@@ -161,6 +184,9 @@
 157,
 192
 ],
+"PhpSpreadsheet:src/PhpSpreadsheet/Reader/Xlsx/ConditionalStyles.php": [
+198
+],
 "PhpSpreadsheet:src/PhpSpreadsheet/Reader/Xlsx/PageSetup.php": [
 49
 ],
@@ -202,6 +228,9 @@
 150,
 326
 ],
+"PhpSpreadsheet:src/PhpSpreadsheet/Shared/StringHelper.php": [
+226
+],
 "PhpSpreadsheet:src/PhpSpreadsheet/Shared/Trend/BestFit.php": [
 299
 ],
@@ -217,6 +246,9 @@
 "PhpSpreadsheet:src/PhpSpreadsheet/Style/Font.php": [
 167
 ],
+"PhpSpreadsheet:src/PhpSpreadsheet/Style/NumberFormat/DateFormatter.php": [
+119
+],
 "PhpSpreadsheet:src/PhpSpreadsheet/Style/NumberFormat/Formatter.php": [
 46,
 118
@@ -241,8 +273,11 @@
 "PhpSpreadsheet:src/PhpSpreadsheet/Worksheet/Worksheet.php": [
 684,
 857,
+1191,
 2106,
-2884
+2884,
+3446,
+3501
 ],
 "PhpSpreadsheet:src/PhpSpreadsheet/Writer/Html.php": [
 461,
@@ -266,6 +301,7 @@
 "PhpSpreadsheet:src/PhpSpreadsheet/Writer/Xls/Parser.php": [
 479,
 1011,
+1172,
 1282,
 1460
 ],
@@ -275,6 +311,7 @@
 ],
 "PhpSpreadsheet:src/PhpSpreadsheet/Writer/Xls/Worksheet.php": [
 223,
+776,
 2373,
 2574,
 2735,
@@ -320,6 +357,8 @@
 "PhpSpreadsheet:src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php": [
 192,
 256,
+589,
+621,
 762,
 861,
 939,
diff --git a/its/ruling/src/integrationTest/resources/expected/monica/php-S1106.json b/its/ruling/src/integrationTest/resources/expected/monica/php-S1106.json
@@ -4142,6 +4142,9 @@
 294,
 306
 ],
+"monica:scripts/tests/server-cc.php": [
+19
+],
 "monica:server.php": [
 15
 ]
diff --git a/its/ruling/src/integrationTest/resources/expected/monica/php-S1311.json b/its/ruling/src/integrationTest/resources/expected/monica/php-S1311.json
@@ -0,0 +1,5 @@
+{
+"monica:app/Models/Contact/Contact.php": [
+47
+]
+}
diff --git a/its/ruling/src/integrationTest/resources/expected/monica/php-S1578.json b/its/ruling/src/integrationTest/resources/expected/monica/php-S1578.json
@@ -2411,6 +2411,9 @@
 "monica:routes/web.php": [
 0
 ],
+"monica:scripts/tests/server-cc.php": [
+0
+],
 "monica:server.php": [
 0
 ]
diff --git a/its/ruling/src/integrationTest/resources/expected/monica/php-S2003.json b/its/ruling/src/integrationTest/resources/expected/monica/php-S2003.json
@@ -4,5 +4,8 @@
 ],
 "monica:public/index.php": [
 24
+],
+"monica:scripts/tests/server-cc.php": [
+26
 ]
 }
diff --git a/its/ruling/src/integrationTest/resources/expected/monica/php-S2007.json b/its/ruling/src/integrationTest/resources/expected/monica/php-S2007.json
@@ -26,6 +26,11 @@
 54,
 55
 ],
+"monica:scripts/tests/server-cc.php": [
+10,
+12,
+28
+],
 "monica:server.php": [
 8
 ]
diff --git a/its/ruling/src/integrationTest/resources/expected/monica/php-S2043.json b/its/ruling/src/integrationTest/resources/expected/monica/php-S2043.json
@@ -1,4 +1,7 @@
 {
+"monica:scripts/tests/server-cc.php": [
+13
+],
 "monica:server.php": [
 9
 ]
diff --git a/its/ruling/src/integrationTest/resources/expected/monica/php-S4833.json b/its/ruling/src/integrationTest/resources/expected/monica/php-S4833.json
@@ -5,6 +5,9 @@
 "monica:public/index.php": [
 38
 ],
+"monica:scripts/tests/server-cc.php": [
+34
+],
 "monica:server.php": [
 19
 ]
diff --git a/its/ruling/src/integrationTest/resources/expected/psysh/php-S1541.json b/its/ruling/src/integrationTest/resources/expected/psysh/php-S1541.json
@@ -1,7 +1,13 @@
 {
+"psysh:src/CodeCleaner/EmptyArrayDimFetchPass.php": [
+45
+],
 "psysh:src/CodeCleaner/FunctionReturnInWriteContextPass.php": [
 46
 ],
+"psysh:src/CodeCleaner/ImplicitReturnPass.php": [
+45
+],
 "psysh:src/CodeCleaner/ListPass.php": [
 42
 ],
@@ -37,6 +43,9 @@
 118,
 259
 ],
+"psysh:src/Command/ShowCommand.php": [
+236
+],
 "psysh:src/Configuration.php": [
 284
 ],
@@ -56,6 +65,7 @@
 32
 ],
 "psysh:src/Input/ShellInput.php": [
+124,
 291
 ],
 "psysh:src/Output/Theme.php": [
@@ -99,6 +109,7 @@
 68
 ],
 "psysh:src/Readline/Hoa/Xcallable.php": [
+85,
 149
 ],
 "psysh:src/Shell.php": [
diff --git a/php-checks/src/main/java/org/sonar/php/checks/FunctionComplexityCheck.java b/php-checks/src/main/java/org/sonar/php/checks/FunctionComplexityCheck.java
@@ -27,11 +27,9 @@
 import org.sonar.plugins.php.api.visitors.PHPSubscriptionCheck;
 import org.sonar.plugins.php.api.visitors.PreciseIssue;
 
-@Rule(key = FunctionComplexityCheck.KEY)
+@Rule(key = "S1541")
 public class FunctionComplexityCheck extends PHPSubscriptionCheck {
 
-  public static final String KEY = "S1541";
-
   private static final String MESSAGE = "The Cyclomatic Complexity of this function %s is %s which is greater than %s authorized.";
 
   public static final int DEFAULT = 20;
diff --git a/php-checks/src/test/java/org/sonar/php/checks/FunctionComplexityCheckTest.java b/php-checks/src/test/java/org/sonar/php/checks/FunctionComplexityCheckTest.java
@@ -26,7 +26,7 @@
 class FunctionComplexityCheckTest {
 
   private static final String FILE_NAME = "FunctionComplexityCheck.php";
-  private FunctionComplexityCheck check = new FunctionComplexityCheck();
+  private final FunctionComplexityCheck check = new FunctionComplexityCheck();
 
   @Test
   void defaultValue() throws Exception {
diff --git a/php-checks/src/test/resources/checks/FunctionComplexityCheck.php b/php-checks/src/test/resources/checks/FunctionComplexityCheck.php
@@ -75,3 +75,9 @@ function nesting() {
   if ($a) {}
   return 1;
 }
+
+function with_elseif() { // Noncompliant [[secondary=+0,+1,+2]]
+  if (true) {
+  } elseif (false) {
+  }
+}
diff --git a/php-frontend/src/main/java/org/sonar/php/metrics/ComplexityVisitor.java b/php-frontend/src/main/java/org/sonar/php/metrics/ComplexityVisitor.java
diff --git a/php-frontend/src/test/java/org/sonar/php/metrics/ComplexityVisitorTest.java b/php-frontend/src/test/java/org/sonar/php/metrics/ComplexityVisitorTest.java

Original file line number	Diff line number	Diff line change
`@@ -66,7 +66,7 @@ void testFlysystem() throws Exception {`
`66`	`66`	`void testMonica() throws Exception {`
`67`	`67`	`// To avoid error: File tests/Unit/Traits/SearchableTest.php can't be indexed twice...`
`68`	`68`	`// the tests directory needs to be excluded`
`69`		`- testProject("monica", "sonar.exclusions", "/tests/");`
	`69`	`+ testProject("monica", "sonar.exclusions", "tests/**");`
`70`	`70`	`}`
`71`	`71`
`72`	`72`	`@Test`
Original file line number	Diff line number	Diff line change
`@@ -3,6 +3,7 @@`
`3`	`3`	`173`
`4`	`4`	`],`
`5`	`5`	`"PHPMailer:src/PHPMailer.php": [`
	`6`	`+1082,`
`6`	`7`	`1222,`
`7`	`8`	`1540,`
`8`	`9`	`1724,`
`@@ -24,5 +25,8 @@`
`24`	`25`	`512,`
`25`	`26`	`730,`
`26`	`27`	`1253`
	`28`	`+],`
	`29`	`+"PHPMailer:test/TestCase.php": [`
	`30`	`+141`
`27`	`31`	`]`
`28`	`32`	`}`
Original file line number	Diff line number	Diff line change
`@@ -23,6 +23,9 @@`
`23`	`23`	`"PhpSpreadsheet:src/PhpSpreadsheet/Writer/Html.php": [`
`24`	`24`	`33`
`25`	`25`	`],`
	`26`	`+"PhpSpreadsheet:src/PhpSpreadsheet/Writer/Xls/Parser.php": [`
	`27`	`+34`
	`28`	`+],`
`26`	`29`	`"PhpSpreadsheet:src/PhpSpreadsheet/Writer/Xls/Worksheet.php": [`
`27`	`30`	`56`
`28`	`31`	`],`
Original file line number	Diff line number	Diff line change
`@@ -4142,6 +4142,9 @@`
`4142`	`4142`	`294,`
`4143`	`4143`	`306`
`4144`	`4144`	`],`
	`4145`	`+"monica:scripts/tests/server-cc.php": [`
	`4146`	`+19`
	`4147`	`+],`
`4145`	`4148`	`"monica:server.php": [`
`4146`	`4149`	`15`
`4147`	`4150`	`]`
-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +{
 +"monica:app/Models/Contact/Contact.php": [
 +47
 +]
 +}
Original file line number	Diff line number	Diff line change
`@@ -2411,6 +2411,9 @@`
`2411`	`2411`	`"monica:routes/web.php": [`
`2412`	`2412`	`0`
`2413`	`2413`	`],`
	`2414`	`+"monica:scripts/tests/server-cc.php": [`
	`2415`	`+0`
	`2416`	`+],`
`2414`	`2417`	`"monica:server.php": [`
`2415`	`2418`	`0`
`2416`	`2419`	`]`
Original file line number	Diff line number	Diff line change
`@@ -4,5 +4,8 @@`
`4`	`4`	`],`
`5`	`5`	`"monica:public/index.php": [`
`6`	`6`	`24`
	`7`	`+],`
	`8`	`+"monica:scripts/tests/server-cc.php": [`
	`9`	`+26`
`7`	`10`	`]`
`8`	`11`	`}`