SONAR-9198 Do not warn when reading property sets with getStringArray

diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/config/DefaultConfiguration.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/config/DefaultConfiguration.java
index a58caf232de08b815bdf9cecf64996029dd1fb08..62e5d10fc0da5e73e868ab9acf3ebd2a6875ecbc 100644 (file)
--- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/config/DefaultConfiguration.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/config/DefaultConfiguration.java
@@ -89,8 +89,9 @@ public abstract class DefaultConfiguration implements Configuration {
   public Optional<String> get(String key) {
     String effectiveKey = definitions.validKey(key);
     PropertyDefinition def = definitions.get(effectiveKey);
-    if (def != null && def.multiValues()) {
-      LOG.warn("Access to the multi-valued property '{}' should be made using 'getStringArray' method. The SonarQube plugin using this property should be updated.", key);
+    if (def != null && (def.multiValues() || !def.fields().isEmpty())) {
+      LOG.warn("Access to the multi-values/property set property '{}' should be made using 'getStringArray' method. The SonarQube plugin using this property should be updated.",
+        key);
     }
     return getInternal(effectiveKey);
   }
@@ -99,8 +100,10 @@ public abstract class DefaultConfiguration implements Configuration {
   public String[] getStringArray(String key) {
     String effectiveKey = definitions.validKey(key);
     PropertyDefinition def = definitions.get(effectiveKey);
-    if (def != null && !def.multiValues()) {
-      LOG.warn("Property '{}' is not declared as multi-valued but was read using 'getStringArray' method. The SonarQube plugin declaring this property should be updated.", key);
+    if (def != null && !def.multiValues() && def.fields().isEmpty()) {
+      LOG.warn(
+        "Property '{}' is not declared as multi-values/property set but was read using 'getStringArray' method. The SonarQube plugin declaring this property should be updated.",
+        key);
     }
     Optional<String> value = getInternal(effectiveKey);
     if (value.isPresent()) {

diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/config/DefaultConfigurationTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/config/DefaultConfigurationTest.java
index 2f65ebceaa01e67b06a3f442344903dcf4476ae4..0432e2511a0e172f36ea66cf7be16ad0adc47361 100644 (file)
--- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/config/DefaultConfigurationTest.java
+++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/config/DefaultConfigurationTest.java
@@ -28,6 +28,7 @@ import org.sonar.api.config.Configuration;
 import org.sonar.api.config.Encryption;
 import org.sonar.api.config.PropertyDefinition;
 import org.sonar.api.config.PropertyDefinitions;
+import org.sonar.api.config.PropertyFieldDefinition;
 import org.sonar.api.utils.log.LogTester;
 import org.sonar.api.utils.log.LoggerLevel;
 
@@ -51,13 +52,15 @@ public class DefaultConfigurationTest {
 
     assertThat(config.get("multiA")).hasValue("a,b");
     assertThat(logTester.logs(LoggerLevel.WARN))
-      .contains("Access to the multi-valued property 'multiA' should be made using 'getStringArray' method. The SonarQube plugin using this property should be updated.");
+      .contains(
+        "Access to the multi-values/property set property 'multiA' should be made using 'getStringArray' method. The SonarQube plugin using this property should be updated.");
 
     logTester.clear();
 
     assertThat(config.getStringArray("single")).containsExactly("foo");
     assertThat(logTester.logs(LoggerLevel.WARN))
-      .contains("Property 'single' is not declared as multi-valued but was read using 'getStringArray' method. The SonarQube plugin declaring this property should be updated.");
+      .contains(
+        "Property 'single' is not declared as multi-values/property set but was read using 'getStringArray' method. The SonarQube plugin declaring this property should be updated.");
 
     logTester.clear();
 
@@ -66,6 +69,26 @@ public class DefaultConfigurationTest {
     assertThat(logTester.logs(LoggerLevel.WARN)).isEmpty();
   }
 
+  @Test
+  public void accessingPropertySetPropertiesShouldBeConsistentWithDeclaration() {
+    Configuration config = new DefaultConfiguration(new PropertyDefinitions(Arrays.asList(
+      PropertyDefinition.builder("props").fields(PropertyFieldDefinition.build("foo1").name("Foo1").build(), PropertyFieldDefinition.build("foo2").name("Foo2").build()).build())),
+      new Encryption(null),
+      mock(AnalysisMode.class),
+      ImmutableMap.of("props", "1,2", "props.1.foo1", "a", "props.1.foo2", "b")) {
+    };
+
+    assertThat(config.get("props")).hasValue("1,2");
+    assertThat(logTester.logs(LoggerLevel.WARN))
+      .contains(
+        "Access to the multi-values/property set property 'props' should be made using 'getStringArray' method. The SonarQube plugin using this property should be updated.");
+
+    logTester.clear();
+
+    assertThat(config.getStringArray("props")).containsExactly("1", "2");
+    assertThat(logTester.logs(LoggerLevel.WARN)).isEmpty();
+  }
+
   @Test
   public void getDefaultValues() {
     Configuration config = new DefaultConfiguration(new PropertyDefinitions(Arrays.asList(