[Enhancement] Adding a Linter to search-processor

[sejli]

Overview

In light of more PRs coming in recently, I've noticed that there are some instances of code where formatting is slightly off or inconsistent with the rest of the code. A linter has been proposed in the past, but has not been followed through with sufficient research.

Implementations

Taking a look at other backend plugins within the opensearch-project, the simplest way to include the same spotlessApply feature as the core repo is to add it in to build.gradle.

performance-analyzer

spotless {
    java {
        licenseHeaderFile(file('license-header'))
        googleJavaFormat('1.12.0').aosp()
        importOrder()
        removeUnusedImports()
        trimTrailingWhitespace()
        endWithNewline()

        // add support for spotless:off and spotless:on tags to exclude sections of code
        toggleOffOn()
    }
}

anomaly-detection

spotless {
    java {
        removeUnusedImports()
        importOrder 'java', 'javax', 'org', 'com'

        eclipse().configFile rootProject.file('.eclipseformat.xml')
    }
}

job-scheduler (job-scheduler uses a formatting.gradle file, but this is the same pattern as the previous examples)

spotless {
        java {
            // Normally this isn't necessary, but we have Java sources in
            // non-standard places
            target '**/*.java'

            removeUnusedImports()
            eclipse().configFile rootProject.file('formatter/formatterConfig.xml')
            trimTrailingWhitespace()
            endWithNewline();

            custom 'Refuse wildcard imports', {
                // Wildcard imports can't be resolved; fail the build
                if (it =~ /\s+import .*\*;/) {
                    throw new AssertionError("Do not use wildcard imports.  'spotlessApply' cannot resolve this issue.")
                }
            }

            // See DEVELOPER_GUIDE.md for details of when to enable this.
            if (System.getProperty('spotless.paddedcell') != null) {
                paddedCell()
            }
        }
        format 'misc', {
            target '*.md', '*.gradle', '**/*.json', '**/*.yaml', '**/*.yml', '**/*.svg'   

            trimTrailingWhitespace()
            endWithNewline()
        }
        format("license", {
            licenseHeaderFile("${rootProject.file("formatter/license-header.txt")}", "package ");
            target("src/*/java/**/*.java")
        })
}

Trying out the first example and running ./gradlew spotlessApply, git status shows many files affected.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   build.gradle
	modified:   src/main/java/org/opensearch/search/relevance/SearchRelevancePlugin.java
	modified:   src/main/java/org/opensearch/search/relevance/actionfilter/SearchActionFilter.java
	modified:   src/main/java/org/opensearch/search/relevance/client/OpenSearchClient.java
	modified:   src/main/java/org/opensearch/search/relevance/configuration/ConfigurationUtils.java
	modified:   src/main/java/org/opensearch/search/relevance/configuration/Constants.java
	modified:   src/main/java/org/opensearch/search/relevance/configuration/ResultTransformerConfiguration.java
	modified:   src/main/java/org/opensearch/search/relevance/configuration/ResultTransformerConfigurationFactory.java
	modified:   src/main/java/org/opensearch/search/relevance/configuration/SearchConfigurationExtBuilder.java
	modified:   src/main/java/org/opensearch/search/relevance/configuration/TransformerConfiguration.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/ResultTransformer.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/TransformerType.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/KendraIntelligentRanker.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/client/KendraClientSettings.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/client/KendraHttpClient.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/client/SimpleAwsErrorHandler.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/client/SimpleResponseHandler.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/configuration/Constants.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/configuration/KendraIntelligentRankerSettings.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/configuration/KendraIntelligentRankingConfiguration.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/configuration/KendraIntelligentRankingConfigurationFactory.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/model/KendraIntelligentRankingException.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/model/PassageScore.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/model/dto/Document.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/model/dto/RescoreRequest.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/model/dto/RescoreResult.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/model/dto/RescoreResultItem.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/pipeline/KendraRankingResponseProcessor.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/preprocess/BM25Scorer.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/preprocess/PassageGenerator.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/preprocess/QueryParser.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/preprocess/SentenceSplitter.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/preprocess/TextTokenizer.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/PersonalizeRankingResponseProcessor.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/client/PersonalizeClient.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/client/PersonalizeClientSettings.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/client/PersonalizeCredentialsProviderFactory.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/configuration/Constants.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/configuration/PersonalizeIntelligentRankerConfiguration.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/requestparameter/PersonalizeRequestParameterUtil.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/requestparameter/PersonalizeRequestParameters.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/requestparameter/PersonalizeRequestParametersExtBuilder.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/reranker/PersonalizedRanker.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/reranker/PersonalizedRankerFactory.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/reranker/impl/AmazonPersonalizedRankerImpl.java
	modified:   src/main/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/utils/ValidationUtil.java
	modified:   src/test/java/org/opensearch/search/relevance/SearchRelevancePluginIT.java
	modified:   src/test/java/org/opensearch/search/relevance/SearchRelevanceTests.java
	modified:   src/test/java/org/opensearch/search/relevance/actionfilter/SearchActionFilterTests.java
	modified:   src/test/java/org/opensearch/search/relevance/configuration/SearchConfigurationExtBuilderTests.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/KendraIntelligentRankerTests.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/client/KendraClientSettingsTests.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/client/KendraHttpClientTests.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/client/KendraIntelligentClientTests.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/client/SimpleAwsErrorHandlerTests.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/configuration/KendraIntelligentRankingConfigurationTests.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/model/KendraIntelligentRankingExceptionTests.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/model/dto/DocumentTests.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/pipeline/KendraRankingResponseProcessorTests.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/preprocess/BM25ScorerTests.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/preprocess/PassageGeneratorTests.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/preprocess/QueryParserTests.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/preprocess/SentenceSplitterTests.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/preprocess/TextTokenizerTests.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/PersonalizeRankingResponseProcessorTests.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/client/PersonalizeClientSettingsTests.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/client/PersonalizeClientTests.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/client/PersonalizeCredentialsProviderFactoryTests.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/configuration/PersonalizeIntelligentRankerConfigurationTests.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/ranker/PersonalizeRankerFactoryTests.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/ranker/impl/AmazonPersonalizeRankerImplTests.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/requestparameter/PersonalizeRequestParameterUtilTests.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/requestparameter/PersonalizeRequestParametersExtBuilderTests.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/utils/PersonalizeClientSettingsTestUtil.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/utils/PersonalizeRuntimeTestUtil.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/utils/SearchTestUtil.java
	modified:   src/test/java/org/opensearch/search/relevance/transformer/personalizeintelligentranking/utils/ValidationUtilTests.java
	modified:   src/yamlRestTest/java/org/opensearch/search/relevance/SearchRelevanceClientYamlTestSuiteIT.java

My proposed course of action is to:

1. Identify the formatting parameters we want to use for search-processor
2. Merge the change to the build.gradle
3. Run spotlessApply, then create another PR
4. Create a workflow that verifies that PRs will now be formatted correctly

This way, we can separate the addition to build.gradle and a repository-wide application of spotlessApply.

[msfroh]

This way, we can separate the addition to build.gradle and a repository-wide application of spotlessApply.

I'm not sure that really helps, and would just leave us temporarily broken.

IMO, we should just push it through with a single commit, since spotlessApply will reformat the code for us. I personally trust Spotless to reformat the code without affecting behavior (since it's never broken me on OpenSearch core).

I suspect that folks contributing to this repo aren't very opinionated about formatting rules and would accept just using e.g. the formatting rules from OpenSearch core: https://github.com/opensearch-project/OpenSearch/blob/a3baa68b7b014520d257efbb0d4b13f66e134d12/gradle/formatting.gradle#L62