PLUGINAPI-190 Promote hidden to a first-class flag on PropertyDefinition (#290)

Hidden can now be used with any scope, and carry the visibility of the property independently. property configuration through annotation is conserved, but not updated to support usage of hidden directly.

Author: steve-marion-sonarsource

plugin-api/src/main/java/org/sonar/api/config/PropertyDefinition.java

@@ -136,6 +136,7 @@ private String getKey() {
   private final String category;
   private final List<ConfigScope> configScopes;
   private final boolean global;
+  private final boolean hidden;
   private final boolean multiValues;
   private final String deprecatedKey;
   private final List<PropertyFieldDefinition> fields;
@@ -153,6 +154,7 @@ private PropertyDefinition(Builder builder) {
     this.category = builder.category;
     this.subCategory = builder.subCategory;
     this.global = builder.global;
+    this.hidden = builder.hidden;
     this.type = builder.type;
     this.options = builder.options;
     this.multiValues = builder.multiValues;
@@ -190,9 +192,19 @@ static PropertyDefinition create(Property annotation) {
       configScopes.add(ConfigScope.MODULE);
     }
     if (annotation.global()) {
-      builder.onConfigScopes(configScopes);
+      if (!configScopes.isEmpty()) {
+        // visible and settable at instance and scope level
+        builder.onConfigScopes(configScopes);
+      }
+      // else default state, visible and settable at instance level only
     } else {
-      builder.onlyOnConfigScopes(configScopes);
+      if (!configScopes.isEmpty()) {
+        // visible and settable at scope only
+        builder.onlyOnConfigScopes(configScopes);
+      } else {
+        // legacy pattern for not visible but settable at instance level only
+        builder.hidden();
+      }
     }
     return builder.build();
   }
@@ -344,6 +356,14 @@ public boolean global() {
     return global;
   }
 
+  /**
+   * Hidden properties are not discoverable, and typically don't appear in the UI.
+   * They can only be configured via the API.
+   */
+  public boolean hidden() {
+    return hidden;
+  }
+
   public boolean multiValues() {
     return multiValues;
   }
@@ -667,11 +687,7 @@ public PropertyDefinition build() {
       checkArgument(!isEmpty(key), "Key must be set");
       fixType(key, type);
       checkArgument(onConfigScopes.isEmpty() || onlyOnConfigScopes.isEmpty(), "Cannot use both forQualifiers and onlyForQualifiers");
-      checkArgument(!hidden || (onConfigScopes.isEmpty() && onlyOnConfigScopes.isEmpty()), "Cannot be hidden and defining qualifiers on which to display");
       checkArgument(!JSON.equals(type) || !multiValues, "Multivalues are not allowed to be defined for JSON-type property.");
-      if (hidden) {
-        global = false;
-      }
       if (!fields.isEmpty()) {
         type = PROPERTY_SET;
       }

plugin-api/src/test/java/org/sonar/api/config/PropertyDefinitionTest.java

@@ -96,6 +96,19 @@ public void should_create_from_annotation() {
     assertThat(def.configScopes()).containsOnly(ConfigScope.PROJECT, ConfigScope.MODULE);
     assertThat(def.multiValues()).isTrue();
     assertThat(def.fields()).isEmpty();
+    assertThat(def.hidden()).isFalse();
+  }
+
+  @Test
+  public void should_translate_legacy_global_false_idiom_to_hidden() {
+    Properties props = AnnotationUtils.getAnnotation(LegacyHiddenIdiom.class, Properties.class);
+    Property prop = props.value()[0];
+
+    PropertyDefinition def = PropertyDefinition.create(prop);
+
+    assertThat(def.hidden()).isTrue();
+    assertThat(def.global()).isTrue();
+    assertThat(def.configScopes()).isEmpty();
   }
 
   @Test
@@ -108,7 +121,34 @@ public void should_create_hidden_property() {
     assertThat(def.key()).isEqualTo("hello");
     assertThat(def.qualifiers()).isEmpty();
     assertThat(def.configScopes()).isEmpty();
+    assertThat(def.global()).isTrue();
+    assertThat(def.hidden()).isTrue();
+  }
+
+  @Test
+  public void should_create_hidden_property_with_on_config_scopes() {
+    PropertyDefinition def = PropertyDefinition.builder("hello")
+      .name("Hello")
+      .hidden()
+      .onConfigScopes(ConfigScope.PROJECT)
+      .build();
+
+    assertThat(def.hidden()).isTrue();
+    assertThat(def.global()).isTrue();
+    assertThat(def.configScopes()).containsExactly(ConfigScope.PROJECT);
+  }
+
+  @Test
+  public void should_create_hidden_property_with_only_on_config_scopes() {
+    PropertyDefinition def = PropertyDefinition.builder("hello")
+      .name("Hello")
+      .hidden()
+      .onlyOnConfigScopes(ConfigScope.PROJECT)
+      .build();
+
+    assertThat(def.hidden()).isTrue();
     assertThat(def.global()).isFalse();
+    assertThat(def.configScopes()).containsExactly(ConfigScope.PROJECT);
   }
 
   @Test
@@ -125,6 +165,7 @@ public void should_create_property_with_default_values() {
     assertThat(def.description()).isEmpty();
     assertThat(def.type()).isEqualTo(PropertyType.STRING);
     assertThat(def.global()).isTrue();
+    assertThat(def.hidden()).isFalse();
     assertThat(def.qualifiers()).isEmpty();
     assertThat(def.configScopes()).isEmpty();
     assertThat(def.multiValues()).isFalse();
@@ -150,6 +191,7 @@ public void should_create_from_annotation_default_values() {
     assertThat(def.configScopes()).isEmpty();
     assertThat(def.multiValues()).isFalse();
     assertThat(def.fields()).isEmpty();
+    assertThat(def.hidden()).isFalse();
   }
 
   @Test
@@ -320,22 +362,6 @@ public void should_not_create_json_multivalue() {
       .hasMessage("Multivalues are not allowed to be defined for JSON-type property.");
   }
 
-  @Test
-  public void should_not_authorize_defining_on_qualifiers_and_hidden() {
-    Builder builder = PropertyDefinition.builder("foo").name("foo").onQualifiers(org.sonar.api.resources.Qualifiers.PROJECT).hidden();
-    assertThatThrownBy(builder::build)
-      .isInstanceOf(IllegalArgumentException.class)
-      .hasMessage("Cannot be hidden and defining qualifiers on which to display");
-  }
-
-  @Test
-  public void should_not_authorize_defining_ony_on_qualifiers_and_hidden() {
-    Builder builder = PropertyDefinition.builder("foo").name("foo").onlyOnQualifiers(org.sonar.api.resources.Qualifiers.PROJECT).hidden();
-    assertThatThrownBy(builder::build)
-      .isInstanceOf(IllegalArgumentException.class)
-      .hasMessage("Cannot be hidden and defining qualifiers on which to display");
-  }
-
   @Test
   public void should_not_authorize_defining_on_qualifiers_and_only_on_qualifiers() {
     Builder builder = PropertyDefinition.builder("foo").name("foo").onQualifiers(org.sonar.api.resources.Qualifiers.MODULE)
@@ -470,4 +496,8 @@ static class WithPropertySet {
   static class DefaultValues {
   }
 
+  @Properties(@Property(key = "hello", name = "Hello", global = false))
+  static class LegacyHiddenIdiom {
+  }
+
 }