Skip to content

Commit

Permalink
Tooling to generate Checkstyle and Spotbugs suppression files (#38357)
Browse files Browse the repository at this point in the history
Tooling to generate Checkstyle and Spotbugs suppression files
  • Loading branch information
alzimmermsft authored Jan 19, 2024
1 parent 9f43fda commit 07ed59b
Show file tree
Hide file tree
Showing 8 changed files with 569 additions and 72 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -249,19 +249,6 @@ the main ServiceBusClientBuilder. -->
<!-- Empty while loop waiting for Reactor stream completion -->
<suppress checks="EmptyBlock" files="com.azure.storage.blob.batch.BlobBatch.java"/>

<!-- Azure Search test models -->
<suppress checks="MethodName|MemberName|ParameterName|VisibilityModifier"
files="com.azure.search.documents.test.environment.models.*.java"/>

<!-- Suppress checks for AutoRest generated Search code -->
<!-- Hand-written classes should be specified here to be checked -->
<suppress checks="."
files=".*[/\\]search[/\\]documents[/\\]implementation[/\\](converters|models)[/\\]"/>
<suppress checks="."
files=".*[/\\]search[/\\]documents[/\\]indexes[/\\]implementation[/\\]models[/\\]"/>
<suppress checks="."
files=".*[/\\]search[/\\]documents[/\\]models[/\\]((?!(IndexBatch|ValueFacetResult|RangeFacetResult|SearchableField|SimpleField|IndexBatchException|ScoringParameter)).*)"/>

<suppress checks="."
files=".*[/\\]webpubsub[/\\]implementation[/\\]"/>
<suppress checks="."
Expand Down Expand Up @@ -413,14 +400,6 @@ the main ServiceBusClientBuilder. -->
<!-- Suppress DigitalTwins for now -->
<suppress checks="com.azure.tools.checkstyle.checks.ServiceClient" files="[/\\]azure-digitaltwins-core[/\\]src[/\\]main[/\\]java[/\\]com[/\\]azure[/\\]digitaltwins[/\\]core[/\\](DigitalTwinsClient|DigitalTwinsAsyncClient).java"/>

<!-- Suppress non-conforming service builder and service clients -->
<!-- The clients don't end with 'Client' but instead with 'Sender' -->
<suppress checks="com.azure.tools.checkstyle.checks.ServiceClientBuilderCheck"
files="com.azure.search.documents.SearchClientBuilder"
lines="511,545,559,572"/>
<suppress checks="com.azure.tools.checkstyle.checks.ServiceClientCheck"
files="com.azure.search.documents.(SearchIndexingBufferedAsyncSender|SearchIndexingBufferedSender)"/>

<!-- Allow Attestation Service use of OpenTelemetry -->
<suppress checks="IllegalImport" files=".*[/\\]src[/\\]test[/\\]java[/\\]com[/\\]azure[/\\]security[/\\]attestation[/\\](AttestationClientTestBase|AttestationTest).java"/>

Expand Down Expand Up @@ -452,9 +431,6 @@ the main ServiceBusClientBuilder. -->
<suppress checks="com.azure.tools.checkstyle.checks.HttpPipelinePolicyCheck"
files="com.azure.communication.callingserver.implementation.RedirectPolicy.java"/>

<!-- Suppress VisibilityModifier in MemberConverterImplTests and FieldBuilderTests.-->
<suppress checks="VisibilityModifier" files="com.azure.search.documents.indexes.FieldBuilderTests.java"/>

<!-- These APIs have already leaked implementation and cannot be changed -->
<suppress checks="com.azure.tools.checkstyle.checks.NoImplInPublicAPI" files="com.azure.messaging.eventhubs.LogPartitionProcessor"/>

Expand Down Expand Up @@ -490,8 +466,6 @@ the main ServiceBusClientBuilder. -->

<suppress checks="com.azure.tools.checkstyle.checks.ThrowFromClientLoggerCheck" files=".*(DeviceCodeCredential|InteractiveBrowserCredential|AzureCliCredential|AzureDeveloperCliCredential|ClientCertificateCredential|ClientSecretCredential|EnvironmentCredential|OnBehalfOfCredential|WorkloadIdentityCredential|ChainedTokenCredential|ClientAssertionCredential|UsernamePasswordCredential).java"/>

<suppress checks="com.azure.tools.checkstyle.checks.EnforceFinalFieldsCheck" files="com.azure.search.documents.indexes.models.SynonymMap"/>

<!-- The build tool plugin checkstyle exceptions -->
<suppress checks="com.azure.tools.checkstyle.checks.ExternalDependencyExposedCheck"
files="com.azure.sdk.build.tool.*\.java"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -614,10 +614,6 @@
<Class name="com.azure.storage.blob.nio.AzureBlobFileAttributes"/>
<Method name="isAccessTierInferred"/>
</And>
<And>
<Class name="com.azure.search.documents.indexes.models.EntityRecognitionSkill"/>
<Method name="areTypelessEntitiesIncluded"/>
</And>
</Or>
<Bug pattern="NP_BOOLEAN_RETURN_NULL"/>
</Match>
Expand Down Expand Up @@ -2044,24 +2040,6 @@
REC_CATCH_EXCEPTION"/>
</Match>

<!-- The field is designed to be unread and for serialization purpose. -->
<Match>
<Class name="~com\.azure\.search\.documents\.indexes\.models\.(EdgeNGramTokenFilter|KeywordTokenizer|LuceneStandardTokenizer|NGramTokenFilter|SynonymMap)"/>
<Bug pattern="URF_UNREAD_FIELD"/>
</Match>

<!-- Use static final ObjectMapper property to save the cost of ObjectMapper initialization. -->
<Match>
<Class name="com.azure.search.documents.SearchAsyncClient"/>
<Bug pattern="SIC_INNER_SHOULD_BE_STATIC_ANON"/>
</Match>

<!-- Use the property to take the custom serializer from SearchClient. -->
<Match>
<Class name="~com\.azure\.search\.documents\.models\.(SearchResult|SuggestResult)"/>
<Bug pattern="UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR,SIC_INNER_SHOULD_BE_STATIC_ANON"/>
</Match>

<!-- Setup in Identity Test classes, required for the tests to work properly.-->
<Match>
<Class name="com.azure.identity.ClientCertificateCredentialTest"/>
Expand Down Expand Up @@ -2193,16 +2171,6 @@
<Bug pattern="SIC_INNER_SHOULD_BE_STATIC_ANON"/>
</Match>

<Match>
<Class name="~com\.azure\.search.*"/>
<Or>
<Field name="odataType"/>
<Field name="type"/>
</Or>
<Bug pattern="URF_UNREAD_FIELD"/>
</Match>


<Match>
<Or>
<Class name="com.azure.aot.graalvm.support.netty.implementation.features.TargetIoNettyHandlerSslJdkAlpnApplicationProtocolNegotiatorAlpnWrapper"/>
Expand Down Expand Up @@ -2244,19 +2212,6 @@
DM_STRING_TOSTRING"/>
</Match>

<!-- Test classes that create clients in setup instead of constructor. -->
<Match>
<Bug pattern="UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR"/>
<Or>
<Class name="com.azure.search.documents.GeographyPointTests"/>
<Field name="searchAsyncClient"/>
</Or>
<Or>
<Class name="com.azure.search.documents.GeographyPointTests"/>
<Field name="searchClient"/>
</Or>
</Match>

<!-- Build tool suppressions -->
<Match>
<Class name="com.azure.sdk.build.tool.ReportGenerator"/>
Expand Down
2 changes: 1 addition & 1 deletion eng/pipelines/scripts/Get-Linting-Commands.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ if ($revapiSourceChanged -or $revapiConfigChanged) {

$spotbugsConfigChanged = (git diff $TargetBranch $SourceBranch --name-only --relative -- "${baseDiffDirectory}/resources/spotbugs/*").Count -gt 0
if ($spotbugsConfigChanged) {
$lintingGoals += ' spotbugs:check'
$lintingGoals += ' spotbugs:check spotbugs:spotbugs'
}

Write-Host "Using linting goals '${lintingGoals}'"
Expand Down
228 changes: 228 additions & 0 deletions eng/scripts/linting_suppression_generator.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,228 @@
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License.

# Use case: Generates Checkstyle (checkstyle-suppressions.xml) and Spotbugs (spotbugs-exclude.xml) suppression files
# for the Java SDK.
#
# To use this tool, run the following command:
#
# python linting_suppression_generator.py --project-folder <project folder>
#
# For example:
#
# python linting_suppression_generator.py --project-folder sdk/identity/azure-identity
#
# This will generate, or update, the suppression files in the project root directory.
#
# This script only supports running against a single project. It won't walk any subdirectories looking for
# multiple projects.

import argparse
import os
import subprocess
from typing import Dict, List, Set, Tuple
import xml.etree.ElementTree as ET

# From this file get to the root path of the repo.
root_path = os.path.normpath(os.path.abspath(__file__) + '/../../../')

def generate_suppression_files(project_folder: str):
project_folder = os.path.join(root_path, project_folder)
if not os.path.exists(project_folder):
print('The project folder does not exist: ' + project_folder)
return

# Check if the project folder is a Java project.
if not os.path.exists(os.path.join(project_folder, 'pom.xml')):
print('The project folder does not contain a pom.xml file: ' + project_folder)
return

generate_linting_violations(project_folder)

generate_checkstyle_suppression_file(project_folder)
generate_spotbugs_suppression_file(project_folder)

def generate_linting_violations(project_folder: str):
# Run mvn checkstyle:check spotbugs:check spotbugs:spotbugs -f <project_folder> "-Dcheckstyle.failOnViolation=false" "-Dcheckstyle.failsOnError=false" "-Dspotbugs.failOnError=false" "-Dcheckstyle.suppressionsLocation=" "-Dspotbugs.excludeFilterFile="
# This will generate the following files:
# target/checkstyle-result.xml
# target/spotbugs.xml
subprocess.run(f'mvn checkstyle:check spotbugs:check spotbugs:spotbugs -f {project_folder} "-Dcheckstyle.failOnViolation=false" "-Dcheckstyle.failsOnError=false" "-Dspotbugs.failOnError=false" "-Dcheckstyle.suppressionsLocation=" "-Dspotbugs.excludeFilterFile="', shell = True)

def generate_checkstyle_suppression_file(project_folder: str):
# Get the path to the checkstyle violations file.
checkstyle_violations_file = os.path.join(project_folder, 'target/checkstyle-result.xml')
if not os.path.exists(checkstyle_violations_file):
print('No Checkstyle violations file was found at: ' + checkstyle_violations_file)
return

# Checkstyle violations are stored in the following format:
# <checkstyle>
# <file name="...">
# <error line="..." column="..." severity="..." message="..." source="..."/>
# ...
# </file>
# </checkstyle>
#
# Where the file name is the full path to the file on disk.
#
# Parsing the violations will be stored in the following structure:
#
# Dict(source, Set(classname))
#
# Where the file_name will be turned into the Java file path (ex: /src/main/java/com/azure/.../MyClass.java -> com.azure...MyClass.java),
# the message will be left as is, and the source will clean the built-in checks to just their name
# (ex: com.puppycrawl.tools.checkstyle.checks.javadoc.MissingJavadocMethodCheck -> MissingJavadocMethodCheck).
violations: Dict[str, Set[str]] = dict()

tree: ET.ElementTree = ET.parse(checkstyle_violations_file)
# root will be the <checkstyle> element.
file_elements: List[ET.Element] = tree.getroot().findall('file')

for file_element in file_elements:
file_name = os.path.normpath(file_element.attrib['name']).replace('\\', '/')

# Sanitize the file name by looking for the first instance of src/main/java, src/samples/java, or src/test/java.
# If none of those are found, then look for the first instance of azure-sdk-for-java.
if file_name.find('src/main/java') != -1:
file_name = file_name[file_name.find('src/main/java') + len('src/main/java/'):]
file_name = file_name.replace('/', '.')
elif file_name.find('src/samples/java') != -1:
file_name = file_name[file_name.find('src/samples/java') + len('src/samples/java/'):]
file_name = file_name.replace('/', '.')
elif file_name.find('src/test/java') != -1:
file_name = file_name[file_name.find('src/test/java') + len('src/test/java/'):]
file_name = file_name.replace('/', '.')
else:
file_name = file_name[file_name.find('azure-sdk-for-java') + len('azure-sdk-for-java/'):]

errors: List[ET.Element] = file_element.findall('error')
for error in errors:
source = error.attrib['source']
if source.startswith('com.puppycrawl'):
source = source[source.rfind('.') + 1:]

if source not in violations:
violations[source] = set()

violations[source].add(file_name)

# Now that we have the violations, we can generate the suppression file.
# The format of the suppression file is as follows:
#
# <?xml version="1.0" encoding="UTF-8"?>
# <!DOCTYPE suppressions PUBLIC "-//Checkstyle//DTD SuppressionFilter Configuration 1.2//EN" "https://checkstyle.org/dtds/suppressions_1_2.dtd">
# <suppressions>
# <suppress files="..." checks="..."/>
# </suppressions>
with open(file=os.path.join(project_folder, 'checkstyle-suppressions.xml'), mode='w') as checkstyle_suppressions:
checkstyle_suppressions.write('<?xml version="1.0" encoding="UTF-8"?>\n')
checkstyle_suppressions.write('<!DOCTYPE suppressions PUBLIC "-//Checkstyle//DTD SuppressionFilter Configuration 1.2//EN" "https://checkstyle.org/dtds/suppressions_1_2.dtd">\n')
checkstyle_suppressions.write('<!-- This file is generated by the /eng/scripts/linting_suppression_generator.py script. -->\n\n')

checkstyle_suppressions.write('<suppressions>\n')

for violation in sorted(violations.items(), key=lambda x: x[0]):
files = sorted(violation[1])
for file in files:
checkstyle_suppressions.write(f' <suppress files="{file}" checks="{violation[0]}" />\n')

checkstyle_suppressions.write('</suppressions>\n')

def generate_spotbugs_suppression_file(project_folder: str):
# Get the path to the spotbugs violations file.
spotbugs_violations_file = os.path.join(project_folder, 'target/spotbugs.xml')
if not os.path.exists(spotbugs_violations_file):
print('No Spotbugs violations file was found at: ' + spotbugs_violations_file)
return

# Spotbugs violations are stored in the following format:
# <BugCollection>
# <file classname="...">
# <BugInstance type='...' priority='...' category='...' message='...' lineNumber='...' />
# ...
# </file>
# </BugCollection>
#
# # Parsing the violations will be stored in the following structure:
#
# Dict(type, Set(classname))
violations: Dict[str, Set[str]] = dict()

tree: ET.ElementTree = ET.parse(spotbugs_violations_file)
# root will be the <checkstyle> element.
file_elements: List[ET.Element] = tree.getroot().findall('file')

for file_element in file_elements:
classname = file_element.attrib['classname']

errors: List[ET.Element] = file_element.findall('BugInstance')
for error in errors:
type = error.attrib['type']

if type not in violations:
violations[type] = set()

violations[type].add(classname)

# Now that we have the violations, we can generate the suppression file.
# The format of the suppression file is as follows:
#
# <?xml version="1.0" encoding="UTF-8"?>
# <FindBugsFilter xmlns="https://github.com/spotbugs/filter/3.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
# xsi:schemaLocation="https://github.com/spotbugs/filter/3.0.0 https://raw.githubusercontent.com/spotbugs/spotbugs/3.1.0/spotbugs/etc/findbugsfilter.xsd">
# <Match>
# <Bug pattern="..." />
# <Class name="..." />
# </Match>
# </FindBugsFilter>
#
# If the same bug applies to multiple classes, then the <Class> element can be repeated:
#
# <Match>
# <Bug pattern="..." />
# <Or>
# <Class name="..." />
# <Class name="..." />
# ...
# </Or>
# </Match>
#
# Before each class an XML comment will be added with the message and line number of the violation.
# This is to make it easier to understand what the violation is for, and how to remedy it.
with open(file=os.path.join(project_folder, 'spotbugs-exclude.xml'), mode='w') as spotbugs_suppressions:
spotbugs_suppressions.write('<?xml version="1.0" encoding="UTF-8"?>\n\n')
spotbugs_suppressions.write('<FindBugsFilter xmlns="https://github.com/spotbugs/filter/3.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"\n')
spotbugs_suppressions.write(' xsi:schemaLocation="https://github.com/spotbugs/filter/3.0.0 https://raw.githubusercontent.com/spotbugs/spotbugs/3.1.0/spotbugs/etc/findbugsfilter.xsd">\n')

for violation in sorted(violations.items(), key=lambda x: x[0]):
spotbugs_suppressions.write(' <Match>\n')
spotbugs_suppressions.write(f' <Bug pattern="{violation[0]}" />\n')

classnames = sorted(violation[1])

if len(classnames) == 1:
# If there is only one class, then we can just write it out.
spotbugs_suppressions.write(f' <Class name="{classnames.pop()}" />\n')
else:
# If there are multiple classes, then we need to use the <Or> element.
spotbugs_suppressions.write(f' <Or>\n')
for classname in classnames:
spotbugs_suppressions.write(f' <Class name="{classname}" />\n')
spotbugs_suppressions.write(f' </Or>\n')

spotbugs_suppressions.write(' </Match>\n')

spotbugs_suppressions.write('</FindBugsFilter>\n')

def main():
parser = argparse.ArgumentParser(description='Generate Checkstyle and Spotbugs suppression files for a project.')
parser.add_argument('--project-folder', '-pf', help='The project to generate suppression files for.')
args = parser.parse_args()
if args.project_folder:
generate_suppression_files(args.project_folder)
else:
print('Please provide a project name.')

if __name__ == '__main__':
main()
Loading

0 comments on commit 07ed59b

Please sign in to comment.