Improved handling of different flavours of library

src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/Main.java

@@ -8,6 +8,7 @@
 import com.fasterxml.jackson.annotation.JsonInclude;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.ObjectWriter;
+import com.fasterxml.jackson.databind.exc.InvalidFormatException;
 
 import java.io.File;
 import java.io.FileInputStream;
@@ -164,8 +165,6 @@ private static void processJavaSourcesJar(File inputFile, APIListing apiListing)
         }
         System.out.println("  Using '" + apiListing.getLanguageVariant() + "' for the language variant");
 
-        final Analyser analyser = new JavaASTAnalyser(apiListing);
-
         // Read all files within the jar file so that we can create a list of files to analyse
         final List<Path> allFiles = new ArrayList<>();
         try (FileSystem fs = FileSystems.newFileSystem(inputFile.toPath(), Main.class.getClassLoader())) {
@@ -179,6 +178,9 @@ private static void processJavaSourcesJar(File inputFile, APIListing apiListing)
                 apiListing.setApiViewProperties(properties);
                 System.out.println("  Found apiview_properties.json file in jar file");
                 System.out.println("    - Found " + properties.getCrossLanguageDefinitionIds().size() + " cross-language definition IDs");
+            } catch (InvalidFormatException e) {
+                System.out.println("  ERROR: Unable to parse apiview_properties.json file in jar file");
+                e.printStackTrace();
             } catch (Exception e) {
                 // this is fine, we just won't have any APIView properties to read in
                 System.out.println("  No apiview_properties.json file found in jar file - continuing...");
@@ -194,6 +196,7 @@ private static void processJavaSourcesJar(File inputFile, APIListing apiListing)
             });
 
             // Do the analysis while the filesystem is still represented in memory
+            final Analyser analyser = new JavaASTAnalyser(apiListing);
             analyser.analyse(allFiles);
         } catch (Exception e) {
             e.printStackTrace();

src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/JavaASTAnalyser.java

@@ -1,5 +1,6 @@
 package com.azure.tools.apiview.processor.analysers;
 
+import com.azure.tools.apiview.processor.analysers.models.AnnotationRule;
 import com.azure.tools.apiview.processor.analysers.util.MiscUtils;
 import com.azure.tools.apiview.processor.analysers.util.TokenModifier;
 import com.azure.tools.apiview.processor.diagnostics.Diagnostics;
@@ -101,8 +102,24 @@ public class JavaASTAnalyser implements Analyser {
     public static final String MODULE_INFO_KEY = "module-info";
 
     private static final boolean SHOW_JAVADOC = true;
-    private static final Set<String> BLOCKED_ANNOTATIONS =
-        new HashSet<>(Arrays.asList("ServiceMethod", "SuppressWarnings"));
+//    private static final Set<String> BLOCKED_ANNOTATIONS =
+//        new HashSet<>(Arrays.asList("ServiceMethod", "SuppressWarnings"));
+
+    private static final Map<String, AnnotationRule> ANNOTATION_RULE_MAP;
+    static {
+        /*
+         For some annotations, we want to customise how they are displayed. Sometimes, we don't show them in any
+         circumstance. Other times, we want to show them but not their attributes. This map is used to define these
+         customisations. These rules override the default output that APIView will do, based on the location
+         annotation in the code.
+         */
+        ANNOTATION_RULE_MAP = new HashMap<>();
+        ANNOTATION_RULE_MAP.put("ServiceMethod", new AnnotationRule().setHidden(true));
+        ANNOTATION_RULE_MAP.put("SuppressWarnings", new AnnotationRule().setHidden(true));
+
+        // we always want @Metadata annotations to be fully expanded
+        ANNOTATION_RULE_MAP.put("Metadata", new AnnotationRule().setHidden(false).setHideAttributes(false));
+    }
 
     private static final Pattern SPLIT_NEWLINE = Pattern.compile(MiscUtils.LINEBREAK);
 
@@ -1033,12 +1050,27 @@ private void getAnnotations(final NodeWithAnnotations<?> nodeWithAnnotations,
             final boolean showAnnotationProperties,
             final boolean addNewline) {
             Consumer<AnnotationExpr> consumer = annotation -> {
+                // Check the annotation rules map for any overrides
+                final String annotationName = annotation.getName().toString();
+                AnnotationRule annotationRule = ANNOTATION_RULE_MAP.get(annotationName);
+
+                boolean _showAnnotationProperties = showAnnotationProperties;
+                if (annotationRule != null) {
+                    // if there is a rule to hide the annotation...stop now!
+                    if (annotationRule.isHidden()) {
+                        return;
+                    }
+
+                    // we override the showAnnotationProperties flag if the annotation rule specifies it
+                    _showAnnotationProperties = !annotationRule.isHideAttributes();
+                }
+
                 if (addNewline) {
                     addToken(makeWhitespace());
                 }
 
-                addToken(new Token(TYPE_NAME, "@" + annotation.getName().toString(), makeId(annotation, nodeWithAnnotations)));
-                if (showAnnotationProperties) {
+                addToken(new Token(TYPE_NAME, "@" + annotationName, makeId(annotation, nodeWithAnnotations)));
+                if (_showAnnotationProperties) {
                     if (annotation instanceof NormalAnnotationExpr) {
                         addToken(new Token(PUNCTUATION, "("));
                         NodeList<MemberValuePair> pairs = ((NormalAnnotationExpr) annotation).getPairs();
@@ -1071,7 +1103,7 @@ private void getAnnotations(final NodeWithAnnotations<?> nodeWithAnnotations,
                 .stream()
                 .filter(annotationExpr -> {
                     String id = annotationExpr.getName().getIdentifier();
-                    return !BLOCKED_ANNOTATIONS.contains(id) && !id.startsWith("Json");
+                    return !id.startsWith("Json");
                 })
                 .sorted(Comparator.comparing(a -> a.getName().getIdentifier())) // we sort the annotations alphabetically
                 .forEach(consumer);

src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/models/AnnotationRule.java

@@ -0,0 +1,30 @@
+package com.azure.tools.apiview.processor.analysers.models;
+
+import java.util.Set;
+
+public class AnnotationRule {
+
+    private boolean hideAnnotation = false;
+
+    private boolean hideAttributes = false;
+
+    private boolean showOnNewline = false;
+
+    public AnnotationRule setHidden(boolean hidden) {
+        this.hideAnnotation = hidden;
+        return this;
+    }
+
+    public boolean isHidden() {
+        return hideAnnotation;
+    }
+
+    public AnnotationRule setHideAttributes(boolean hidden) {
+        this.hideAttributes = hidden;
+        return this;
+    }
+
+    public boolean isHideAttributes() {
+        return hideAttributes;
+    }
+}

src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/diagnostics/Diagnostics.java

@@ -8,40 +8,52 @@
 import com.azure.tools.apiview.processor.diagnostics.rules.general.*;
 import com.azure.tools.apiview.processor.diagnostics.rules.utils.MiscUtils;
 import com.azure.tools.apiview.processor.model.APIListing;
+import com.azure.tools.apiview.processor.model.Flavor;
 import com.github.javaparser.ast.CompilationUnit;
 
 import java.util.ArrayList;
 import java.util.List;
 import java.util.regex.Pattern;
 
+import static com.azure.tools.apiview.processor.model.Flavor.*;
+
 import static com.azure.tools.apiview.processor.diagnostics.rules.general.IllegalMethodNamesDiagnosticRule.Rule;
 import static com.azure.tools.apiview.processor.diagnostics.rules.RequiredBuilderMethodsDiagnosticRule.DirectSubclassCheckFunction;
 import static com.azure.tools.apiview.processor.diagnostics.rules.RequiredBuilderMethodsDiagnosticRule.ExactTypeNameCheckFunction;
 import static com.azure.tools.apiview.processor.diagnostics.rules.RequiredBuilderMethodsDiagnosticRule.ParameterAllowedTypes;
 import static com.azure.tools.apiview.processor.diagnostics.rules.general.BadAnnotationDiagnosticRule.BadAnnotation;
 
 public class Diagnostics {
-    private static final String AZURE_PACKAGE_PREFIX = "com.azure";
-    private static final String CLIENT_CORE_PACKAGE_PREFIX = "com.generic"; // FIXME need to update this to the correct package prefix
-
     final List<DiagnosticRule> rules;
 
     public Diagnostics(APIListing apiListing) {
         rules = new ArrayList<>();
 
-        System.out.print("  Setting up diagnostics...");
+        System.out.println("  Setting up diagnostics...");
+
+        Flavor flavor = Flavor.getFlavor(apiListing);
 
         // Special rules for com.azure or io.clientcore libraries only
-        if (apiListing.getPackageName().startsWith(AZURE_PACKAGE_PREFIX)) {
-            System.out.println("Applying com.azure specific diagnostics...");
-            addAzureCoreDiagnostics();
-        } else if (apiListing.getPackageName().startsWith(CLIENT_CORE_PACKAGE_PREFIX)) {
-            System.out.println("Applying io.clientcore specific diagnostics...");
-            addClientCoreDiagnostics();
-        } else {
-            System.out.println("Applying general-purpose diagnostics...");
+        switch (flavor) {
+            case AZURE: {
+                System.out.println("    Applying com.azure specific diagnostics...");
+                addAzureCoreDiagnostics();
+                break;
+            }
+            case GENERIC: {
+                System.out.println("    Applying io.clientcore specific diagnostics...");
+                addClientCoreDiagnostics();
+                break;
+            }
+            case UNKNOWN:
+            default: {
+                System.out.println("    Unknown library flavor...");
+                break;
+            }
         }
 
+        System.out.println("    Applying general-purpose diagnostics...");
+
         // general rules applicable in all cases
         rules.add(new UpperCaseEnumValuesDiagnosticRule());
         rules.add(new ImportsDiagnosticRule("com.sun"));
@@ -68,54 +80,54 @@ public Diagnostics(APIListing apiListing) {
     }
 
     private void addAzureCoreDiagnostics() {
-        rules.add(new PackageNameDiagnosticRule(Pattern.compile("^" + AZURE_PACKAGE_PREFIX + "(\\.[a-z0-9]+)+$")));
+        rules.add(new PackageNameDiagnosticRule(Pattern.compile("^" + AZURE.getPackagePrefix() + "(\\.[a-z0-9]+)+$")));
         rules.add(new AzureCoreBuilderTraitsDiagnosticRule());
-        rules.add(new MissingAnnotationsDiagnosticRule(AZURE_PACKAGE_PREFIX));
+        rules.add(new MissingAnnotationsDiagnosticRule(AZURE.getPackagePrefix()));
         rules.add(new AzureCoreFluentSetterReturnTypeDiagnosticRule());
         rules.add(new ServiceVersionDiagnosticRule());
         rules.add(new ExpandableEnumDiagnosticRule("ExpandableStringEnum"));
         rules.add(new MavenPackageAndDescriptionDiagnosticRule());
 
         // common APIs for all builders (below we will do rules for http or amqp builders)
         rules.add(new RequiredBuilderMethodsDiagnosticRule(null)
-                .add("configuration", new ExactTypeNameCheckFunction("Configuration"))
-                .add("clientOptions", new ExactTypeNameCheckFunction("ClientOptions"))
-                .add("connectionString", new ExactTypeNameCheckFunction("String"))
-                .add("credential", new ExactTypeNameCheckFunction(new ParameterAllowedTypes("TokenCredential",
-                        "AzureKeyCredential", "AzureSasCredential", "AzureNamedKeyCredential", "KeyCredential")))
-                .add("endpoint", new ExactTypeNameCheckFunction("String"))
-                .add("serviceVersion", m -> MiscUtils.checkMethodParameterTypeSuffix(m, "ServiceVersion")));
+            .add("configuration", new ExactTypeNameCheckFunction("Configuration"))
+            .add("clientOptions", new ExactTypeNameCheckFunction("ClientOptions"))
+            .add("connectionString", new ExactTypeNameCheckFunction("String"))
+            .add("credential", new ExactTypeNameCheckFunction(new ParameterAllowedTypes("TokenCredential",
+                    "AzureKeyCredential", "AzureSasCredential", "AzureNamedKeyCredential", "KeyCredential")))
+            .add("endpoint", new ExactTypeNameCheckFunction("String"))
+            .add("serviceVersion", m -> MiscUtils.checkMethodParameterTypeSuffix(m, "ServiceVersion")));
         rules.add(new RequiredBuilderMethodsDiagnosticRule("amqp")
-                .add("proxyOptions", new ExactTypeNameCheckFunction("ProxyOptions"))
-                .add("retry", new ExactTypeNameCheckFunction("AmqpRetryOptions"))
-                .add("transportType", new DirectSubclassCheckFunction("AmqpTransportType")));
+            .add("proxyOptions", new ExactTypeNameCheckFunction("ProxyOptions"))
+            .add("retry", new ExactTypeNameCheckFunction("AmqpRetryOptions"))
+            .add("transportType", new DirectSubclassCheckFunction("AmqpTransportType")));
         rules.add(new RequiredBuilderMethodsDiagnosticRule("http")
-                .add("addPolicy", new ExactTypeNameCheckFunction("HttpPipelinePolicy"))
-                .add("httpClient", new ExactTypeNameCheckFunction("HttpClient"))
-                .add("httpLogOptions", new ExactTypeNameCheckFunction("HttpLogOptions"))
-                .add("pipeline", new ExactTypeNameCheckFunction("HttpPipeline"))
-                .add("retryPolicy", new ExactTypeNameCheckFunction("RetryPolicy")));
+            .add("addPolicy", new ExactTypeNameCheckFunction("HttpPipelinePolicy"))
+            .add("httpClient", new ExactTypeNameCheckFunction("HttpClient"))
+            .add("httpLogOptions", new ExactTypeNameCheckFunction("HttpLogOptions"))
+            .add("pipeline", new ExactTypeNameCheckFunction("HttpPipeline"))
+            .add("retryPolicy", new ExactTypeNameCheckFunction("RetryPolicy")));
     }
 
     private void addClientCoreDiagnostics() {
-        rules.add(new PackageNameDiagnosticRule(Pattern.compile("^" + CLIENT_CORE_PACKAGE_PREFIX + "(\\.[a-z0-9]+)+$")));
+//        rules.add(new PackageNameDiagnosticRule(Pattern.compile("^" + GENERIC.getPackagePrefix() + "(\\.[a-z0-9]+)+$")));
         rules.add(new ClientCoreBuilderTraitsDiagnosticRule());
-        rules.add(new MissingAnnotationsDiagnosticRule(CLIENT_CORE_PACKAGE_PREFIX));
+        rules.add(new MissingAnnotationsDiagnosticRule(GENERIC.getPackagePrefix()));
         rules.add(new ClientCoreFluentSetterReturnTypeDiagnosticRule());
         rules.add(new ExpandableEnumDiagnosticRule("ExpandableEnum"));
 
         // common APIs for all builders (below we will do rules for http or amqp builders)
         rules.add(new RequiredBuilderMethodsDiagnosticRule(null)
-                .add("endpoint", new ExactTypeNameCheckFunction("String"))
-                .add("configuration", new ExactTypeNameCheckFunction("Configuration"))
-                .add("credential", new ExactTypeNameCheckFunction(new ParameterAllowedTypes("KeyCredential")))
-                .add("addHttpPipelinePolicy", new ExactTypeNameCheckFunction("HttpPipelinePolicy"))
-                .add("httpClient", new ExactTypeNameCheckFunction("HttpClient"))
-                .add("httpLogOptions", new ExactTypeNameCheckFunction("HttpLogOptions"))
-                .add("httpPipeline", new ExactTypeNameCheckFunction("HttpPipeline"))
-                .add("httpRetryOptions", new ExactTypeNameCheckFunction("RetryOptions"))
-                .add("httpRedirectOptions", new ExactTypeNameCheckFunction("HttpRedirectOptions"))
-                .add("proxyOptions", new ExactTypeNameCheckFunction("ProxyOptions")));
+            .add("endpoint", new ExactTypeNameCheckFunction("String"))
+            .add("configuration", new ExactTypeNameCheckFunction("Configuration"))
+            .add("credential", new ExactTypeNameCheckFunction(new ParameterAllowedTypes("KeyCredential")))
+            .add("addHttpPipelinePolicy", new ExactTypeNameCheckFunction("HttpPipelinePolicy"))
+            .add("httpClient", new ExactTypeNameCheckFunction("HttpClient"))
+            .add("httpLogOptions", new ExactTypeNameCheckFunction("HttpLogOptions"))
+            .add("httpPipeline", new ExactTypeNameCheckFunction("HttpPipeline"))
+            .add("httpRetryOptions", new ExactTypeNameCheckFunction("RetryOptions"))
+            .add("httpRedirectOptions", new ExactTypeNameCheckFunction("HttpRedirectOptions"))
+            .add("proxyOptions", new ExactTypeNameCheckFunction("ProxyOptions")));
     }
 
     private void add(DiagnosticRule rule) {

src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/ApiViewProperties.java

@@ -13,6 +13,9 @@
  */
 public class ApiViewProperties {
 
+    @JsonProperty("flavor")
+    private Flavor flavor;
+
     // This is a map of model names and methods to their TypeSpec definition IDs.
     @JsonProperty("CrossLanguageDefinitionId")
     private final Map<String, String> crossLanguageDefinitionIds = new HashMap<>();
@@ -32,4 +35,8 @@ public Optional<String> getCrossLanguageDefinitionId(String fullyQualifiedName)
     public Map<String, String> getCrossLanguageDefinitionIds() {
         return Collections.unmodifiableMap(crossLanguageDefinitionIds);
     }
+
+    public Flavor getFlavor() {
+        return flavor;
+    }
 }

src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/Flavor.java

@@ -0,0 +1,48 @@
+package com.azure.tools.apiview.processor.model;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+
+public enum Flavor {
+    @JsonProperty("azure")
+    AZURE("com.azure"),
+
+    @JsonProperty("generic")
+    GENERIC("io.clientcore"),
+
+    UNKNOWN(null);
+
+    private final String packagePrefix;
+
+    private Flavor(String packagePrefix) {
+        this.packagePrefix = packagePrefix;
+    }
+
+    public String getPackagePrefix() {
+        return packagePrefix;
+    }
+
+    public static Flavor getFlavor(APIListing apiListing) {
+        Flavor flavor = apiListing.getApiViewProperties().getFlavor();
+        if (flavor != null) {
+            return flavor;
+        }
+
+        // we are reviewing a library that does not have a flavor metadata in its apiview_properties.json file,
+        // so we will use alternate means to determine the flavor.
+
+        // Firstly, check the package name - does it start with one of the known package prefixes?
+        if (apiListing.getPackageName().startsWith(AZURE.packagePrefix)) {
+            return AZURE;
+        } else if (apiListing.getPackageName().startsWith(GENERIC.packagePrefix)) {
+            return GENERIC;
+        }
+
+        // TODO we still don't know the flavor, so the next thing we can do is look at the dependencies of the library
+        // to see if it brings in com.azure or io.clientcore libraries.
+
+        // we've failed - return the unknown flavor
+        return UNKNOWN;
+    }
+
+
+}

src/java/apiview-java-processor/src/test/java/com/azure/tools/apiview/processor/diagnostics/rules/MavenPackageAndDescriptionDiagnosticRuleTests.java → src/java/apiview-java-processor/src/test/java/com/azure/tools/apiview/processor/diagnostics/rules/azure/MavenPackageAndDescriptionDiagnosticRuleTests.java

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
-package com.azure.tools.apiview.processor.diagnostics.rules;
+package com.azure.tools.apiview.processor.diagnostics.rules.azure;
 
 import com.azure.tools.apiview.processor.diagnostics.rules.azure.MavenPackageAndDescriptionDiagnosticRule;
 import com.azure.tools.apiview.processor.model.APIListing;

src/java/apiview-java-processor/src/test/java/com/azure/tools/apiview/processor/diagnostics/rules/NoLocalesInJavadocUrlDiagnosticRuleTests.java → src/java/apiview-java-processor/src/test/java/com/azure/tools/apiview/processor/diagnostics/rules/general/NoLocalesInJavadocUrlDiagnosticRuleTests.java

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
-package com.azure.tools.apiview.processor.diagnostics.rules;
+package com.azure.tools.apiview.processor.diagnostics.rules.general;
 
 import com.azure.tools.apiview.processor.diagnostics.rules.general.NoLocalesInJavadocUrlDiagnosticRule;
 import com.azure.tools.apiview.processor.model.APIListing;