Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce tooling-api #2860

Closed
arturbosch opened this issue Jul 12, 2020 · 9 comments
Closed

Introduce tooling-api #2860

arturbosch opened this issue Jul 12, 2020 · 9 comments
Assignees
Projects
Milestone

Comments

@arturbosch
Copy link
Member

arturbosch commented Jul 12, 2020

Expected Behavior

One module which can be used across all embedders to run detekt.
Configuration is made via a Spec dsl.
The tooling api defines interfaces to run detekt according to the plugin needs.

The goal is to unify using detekt in: cli, gradle-plugin, maven-plugin, bazel-plugin, intellij-plugin, sonar-detekt and the compiler-plugin,

Current Behavior

Every plugin creates internal core and cli classes like ProcessingSettings, DetektFacade and loadDefaultConfig to load and run detekt. Most often CliArgs is used to setup detekt making the cli module more core than core.

Context

When making changes to core or cli there are often breaking change for the Intellij or Sonarqube plugins.

@arturbosch arturbosch added this to the 1.11.0 milestone Jul 12, 2020
@arturbosch arturbosch self-assigned this Jul 12, 2020
@arturbosch
Copy link
Member Author

arturbosch commented Jul 12, 2020

DSL I came up with

Using the tooling-api in the cli module:

internal fun CliArgs.createSpec(output: Appendable, error: Appendable): ProcessingSpec {
    val args = this
    return ProcessingSpec {

        logging {
            debug = args.debug
            outputChannel = output
            errorChannel = error
        }

        project {
            inputPaths = args.inputPaths
            excludes = asPatterns(args.excludes)
            includes = asPatterns(args.includes)
        }

        rules {
            autoCorrect = args.autoCorrect
            activateExperimentalRules = args.failFast
            maxIssuePolicy = RulesSpec.MaxIssuePolicy.NonSpecified // not yet supported; prefer to read from config
            excludeCorrectable = false // not yet supported; loaded from config
            runPolicy = args.toRunPolicy()
        }

        baseline {
            path = args.baseline
            shouldCreateDuringAnalysis = args.createBaseline
        }

        config {
            useDefaultConfig = args.buildUponDefaultConfig
            shouldValidateBeforeAnalysis = false
            knownPatterns = emptyList()
            // ^^ cli does not have these properties yet; specified in yaml config for now
            configPaths = config?.let { MultipleExistingPathConverter().convert(it) } ?: emptyList()
            resources = configResource?.let { MultipleClasspathResourceConverter().convert(it) } ?: emptyList()
        }

        execution {
            parallelParsing = args.parallel
            parallelAnalysis = args.parallel
        }

        extensions {
            disableDefaultRuleSets = args.disableDefaultRuleSets
            fromPaths { args.plugins?.let { MultipleExistingPathConverter().convert(it) } ?: emptyList() }
        }

        reports {
            args.reportPaths.forEach {
                report { it.kind to it.path }
            }
        }

        compiler {
            jvmTarget = args.jvmTarget.description
            languageVersion = args.languageVersion?.versionString
            classpath = args.classpath?.trim()
        }
    }
}

@arturbosch
Copy link
Member Author

arturbosch commented Jul 12, 2020

Detekt interface to run an analysis

/**
 * Instance of detekt.
 *
 * Runs analysis based on [io.github.detekt.tooling.api.spec.ProcessingSpec] configuration.
 */
interface Detekt {

    fun run(): AnalysisResult

    fun run(path: Path): AnalysisResult

    fun run(sourceCode: String, filename: String): AnalysisResult

    fun run(files: Collection<KtFile>, bindingContext: BindingContext): AnalysisResult

    fun run(files: Collection<KtFile>, bindingTrace: BindingTrace): AnalysisResult
}

@arturbosch arturbosch removed this from the 1.11.0 milestone Jul 12, 2020
@BraisGabin
Copy link
Member

BraisGabin commented Jul 12, 2020

Runs analysis based on [io.github.detekt.tooling.api.spec.ProcessingSpec] configuration.

Which ProcessingSpec? There is not ProcessingSpec in this interface. and, do we need 5 different functions? Can't we just have one and then add extension funcitions for the other ones? Or default implementations? This way the core just do it's thing. It doesn't know how the clients use it.

@arturbosch
Copy link
Member Author

Oh, it's the detekt provider interface which needs the ProcessingSpec to construct a Detekt instance.

interface DetektProvider {

    /**
     * Configure a [Detekt] instance based on given [ProcessingSpec].
     */
    fun get(processingSpec: ProcessingSpec): Detekt

The run methods are convenience methods for the clients. Else they have no way to run a detekt analysis just on in-memory sourcecode or with a custom BindingContext. They would need to dig in the core module and use internals.

@arturbosch arturbosch added this to the 1.11.0 milestone Jul 14, 2020
@arturdryomov
Copy link
Contributor

One thing I want to add (since we might use this in the Bazel rule) — please don’t forget about the documentation. Right now we don’t copy-paste Detekt options documentation and redirect to the CLI API description. It would be useful at least for Gradle, Maven, Bazel and CLI to have a glossary of sorts to continue doing this. I. e. instead of documenting option A in a Maven plugin have an ability to state that it configures the Z Detekt option.

@arturbosch arturbosch added this to Next in Roadmap Jul 20, 2020
@arturbosch arturbosch modified the milestones: 1.11.0-RC2, 1.11.0 Aug 9, 2020
Roadmap automation moved this from Next to Done Aug 11, 2020
@arturbosch arturbosch changed the title Introduce tooling-api for all embedders Introduce tooling-api Aug 13, 2020
@snooze92
Copy link

snooze92 commented Oct 18, 2020

Is this tooling API documented anywhere? I am attempting to bump Detekt in Sputnik, but am struggling to find exactly how to replace the previous invocation with using the new API.

Edit: I think I found how to do what I needed there, but documentation would not hurt anyways! :)

@arturbosch
Copy link
Member Author

@snooze92 glad you found it :).
There are some properties of the Spec which do not change any behavior and using the AnalysisResult does require to @OptIn. When we smooth out these edges documentation will follow :).

@snooze92
Copy link

OK, makes sense. You can see my proposal for bumping Detekt in Sputnik, mostly here.

@arturbosch
Copy link
Member Author

OK, makes sense. You can see my proposal for bumping Detekt in Sputnik, mostly here.

Nice to see the tooling-api in action outside of detekt :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Roadmap
  
Done
Development

No branches or pull requests

4 participants