Allow plugins to contribute to the default configuration by BraisGabin · Pull Request #4315 · detekt/detekt

BraisGabin · 2021-11-20T20:05:28Z

We should allow that the detekt plugins could contribute with the configuration of it's rules (Slack comment and #3242. There are two places where we should allow this: when the core request the default configuration and when the user request to generare the configuration file.

With this PR if a plugin author adds a configuration file in their resources at the path config/config.yml detekt would concat it at the end of the configuration file.

This PR doesn't give any support to generate nor keep in sync those configurations files but I think that it should be doable in the future.

Related #3242
Closes #1103

codecov · 2021-12-01T22:46:53Z

Codecov Report

Merging #4315 (86e6526) into main (8a919d9) will decrease coverage by 0.07%.
The diff coverage is 73.91%.

@@             Coverage Diff              @@
##               main    #4315      +/-   ##
============================================
- Coverage     84.35%   84.28%   -0.08%     
- Complexity     3277     3283       +6     
============================================
  Files           473      472       -1     
  Lines         10357    10450      +93     
  Branches       1827     1863      +36     
============================================
+ Hits           8737     8808      +71     
- Misses          668      671       +3     
- Partials        952      971      +19

Impacted Files	Coverage Δ
...otlin/io/gitlab/arturbosch/detekt/cli/CliRunner.kt	`7.14% <0.00%> (ø)`
...b/arturbosch/detekt/core/tooling/AnalysisFacade.kt	`62.85% <37.50%> (-3.81%)`	⬇️
...otlin/io/gitlab/arturbosch/detekt/core/Analyzer.kt	`81.81% <66.66%> (ø)`
...ab/arturbosch/detekt/core/config/Configurations.kt	`84.37% <66.66%> (ø)`
...bosch/detekt/core/tooling/DefaultConfigProvider.kt	`80.00% <77.77%> (ø)`
...ab/arturbosch/detekt/cli/runners/ConfigExporter.kt	`88.88% <100.00%> (+3.17%)`	⬆️
.../arturbosch/detekt/core/config/ConfigValidators.kt	`84.21% <100.00%> (+0.87%)`	⬆️
...kt/core/config/DefaultPropertiesConfigValidator.kt	`100.00% <100.00%> (ø)`
...gitlab/arturbosch/detekt/core/tooling/Lifecycle.kt	`100.00% <100.00%> (ø)`
...detekt/tooling/api/DefaultConfigurationProvider.kt	`100.00% <100.00%> (ø)`
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8a919d9...86e6526. Read the comment docs.

BraisGabin · 2021-12-02T00:10:16Z

It's difficult to unit test this >_<

picklebento · 2021-12-02T03:21:56Z

+        .use { it.copyTo(outputStream) }
+
+    ExtensionFacade(spec.extensionsSpec).pluginLoader
+        .getResourcesAsStream("config/config.yml")


Do you think this path config/config.yml should be configurable? If not, we probably need to add documentation in

I don't think this could be configurable.

We need to document this. But should we document it right now? Or should we wait to release it first? And probably we can create a tool to generate this files from our annotations. The same that we have already for detekt but working for anyone.

If not configurable, should this be exposed in detekt-api so that other plugin authors could refer to this constant. (the prefixing config/ does look a bit awkward)

BraisGabin · 2021-12-02T08:55:13Z

-	public final fun load (Ljava/lang/ClassLoader;)Lio/github/detekt/tooling/api/DefaultConfigurationProvider;
-	public static synthetic fun load$default (Lio/github/detekt/tooling/api/DefaultConfigurationProvider$Companion;Ljava/lang/ClassLoader;ILjava/lang/Object;)Lio/github/detekt/tooling/api/DefaultConfigurationProvider;
+	public final fun load (Lio/github/detekt/tooling/api/spec/ProcessingSpec;Ljava/lang/ClassLoader;)Lio/github/detekt/tooling/api/DefaultConfigurationProvider;
+	public static synthetic fun load$default (Lio/github/detekt/tooling/api/DefaultConfigurationProvider$Companion;Lio/github/detekt/tooling/api/spec/ProcessingSpec;Ljava/lang/ClassLoader;ILjava/lang/Object;)Lio/github/detekt/tooling/api/DefaultConfigurationProvider;


I assume that no one is calling this function nor implementing its own DefaultConfigurationProvider. But this change breaks code and binary compatibility. And I don't know how to implement it without this change.

I think this breaking change is okay if it's highlighted in the changelog.

cortinico · 2021-12-02T09:48:45Z

That's great stuff! Can I ask you to update the PR name to something more meaningful as it will be harder to search & understand in the changelog

BraisGabin · 2021-12-02T09:53:08Z

That's great stuff! Can I ask you to update the PR name to something more meaningful as it will be harder to search & understand in the changelog

You are completely right.

schalkms · 2021-12-26T20:27:47Z

-	public final fun load (Ljava/lang/ClassLoader;)Lio/github/detekt/tooling/api/DefaultConfigurationProvider;
-	public static synthetic fun load$default (Lio/github/detekt/tooling/api/DefaultConfigurationProvider$Companion;Ljava/lang/ClassLoader;ILjava/lang/Object;)Lio/github/detekt/tooling/api/DefaultConfigurationProvider;
+	public final fun load (Lio/github/detekt/tooling/api/spec/ProcessingSpec;Ljava/lang/ClassLoader;)Lio/github/detekt/tooling/api/DefaultConfigurationProvider;
+	public static synthetic fun load$default (Lio/github/detekt/tooling/api/DefaultConfigurationProvider$Companion;Lio/github/detekt/tooling/api/spec/ProcessingSpec;Ljava/lang/ClassLoader;ILjava/lang/Object;)Lio/github/detekt/tooling/api/DefaultConfigurationProvider;


I think this breaking change is okay if it's highlighted in the changelog.

Co-authored-by: M Schalk <30376729+schalkms@users.noreply.github.com>

picklebento

Looks good to me overall. Thank you for building this feature!

Co-authored-by: Chao Zhang <zhangchao6865@gmail.com>

cortinico · 2022-01-03T19:04:24Z

Great stuff! @BraisGabin can you add a paragraph in the UNRELEASED section of the Changelog to explain how this will work for the end user.

BraisGabin · 2022-01-03T21:50:12Z

@cortinico sorry but I don't know where do you want me to write it.

cortinico · 2022-01-05T16:17:09Z

@cortinico sorry but I don't know where do you want me to write it.

I meant here:

detekt/docs/pages/changelog 1.x.x.md

Line 8 in 40efb62

(we should create an #### UNRELEASED section and start populating the Changelog for 1.20.0).

cortinico · 2022-02-26T14:28:23Z

Do we happen to have a page in the documentation about this feature?

BraisGabin · 2022-02-28T15:51:26Z

No, we don't have it. I'll try to write somthing about it. A tool to automate it would be nice too (#4457).

BraisGabin mentioned this pull request Nov 20, 2021

Simplify YamlConfig #4316

Merged

BraisGabin added this to the 1.20.0 milestone Nov 20, 2021

BraisGabin changed the base branch from main to improve-test-default-config November 20, 2021 20:13

Base automatically changed from improve-test-default-config to main November 20, 2021 20:57

BraisGabin force-pushed the configure-config branch from 74fc2b3 to 0d22c7c Compare November 20, 2021 21:37

BraisGabin changed the base branch from main to simplify-yamlconfig November 20, 2021 21:37

Base automatically changed from simplify-yamlconfig to main November 24, 2021 09:47

BraisGabin force-pushed the configure-config branch from 0d22c7c to 1f0f20c Compare November 24, 2021 10:10

BraisGabin force-pushed the configure-config branch from 1f0f20c to 74737bd Compare December 1, 2021 22:30

BraisGabin force-pushed the configure-config branch from 74737bd to ea9eaad Compare December 2, 2021 00:09

BraisGabin force-pushed the configure-config branch 2 times, most recently from 430327f to 627d2c1 Compare December 2, 2021 00:30

picklebento reviewed Dec 2, 2021

View reviewed changes

BraisGabin force-pushed the configure-config branch from 627d2c1 to 2684a57 Compare December 2, 2021 08:51

BraisGabin commented Dec 2, 2021

View reviewed changes

BraisGabin marked this pull request as ready for review December 2, 2021 09:23

BraisGabin force-pushed the configure-config branch from 2684a57 to b2d6f10 Compare December 2, 2021 09:47

cortinico added the notable changes Marker for notable changes in the changelog label Dec 2, 2021

BraisGabin changed the title ~~Configure config~~ Allow plugins to contribute to the default configuration Dec 2, 2021

BraisGabin mentioned this pull request Dec 24, 2021

Detekt deactivates non activated rules by default which causes confusion for detekt extension authors #1103

Closed

schalkms reviewed Dec 26, 2021

View reviewed changes

BraisGabin added 5 commits December 26, 2021 22:58

Add missing tests

3ed9e88

Remove deprecated code

8fef21b

Get Spec in DefaultConfigProvider

64273a4

Remove DefaultConfig

7179b6d

Concat configurations

00a2e2e

Apply suggestions from code review

c6b0b1b

Co-authored-by: M Schalk <30376729+schalkms@users.noreply.github.com>

BraisGabin force-pushed the configure-config branch from 1518949 to c6b0b1b Compare December 26, 2021 22:08

BraisGabin mentioned this pull request Dec 26, 2021

Disable CI for Windows & JDK8 #4403

Merged

picklebento approved these changes Dec 27, 2021

View reviewed changes

Comment thread ...t-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/tooling/DefaultConfigProviderSpec.kt Outdated

Apply suggestions from code review

86e6526

Co-authored-by: Chao Zhang <zhangchao6865@gmail.com>

BraisGabin merged commit a7184d1 into main Jan 2, 2022

BraisGabin deleted the configure-config branch January 2, 2022 17:55

This was referenced Jan 5, 2022

Update changelog #4456

Merged

Create a tool for rule authors to contribute to the default configuration #4457

Open

BraisGabin mentioned this pull request Mar 23, 2022

Fix regression generating configuration #4646

Merged

Uh oh!

Uh oh!

Conversation

BraisGabin commented Nov 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Dec 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

BraisGabin commented Dec 2, 2021

Uh oh!

picklebento Dec 2, 2021

Choose a reason for hiding this comment

Uh oh!

BraisGabin Dec 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

picklebento Dec 27, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

BraisGabin Dec 2, 2021

Choose a reason for hiding this comment

Uh oh!

schalkms Dec 26, 2021

Choose a reason for hiding this comment

Uh oh!

cortinico commented Dec 2, 2021

Uh oh!

BraisGabin commented Dec 2, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

schalkms Dec 26, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

picklebento left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cortinico commented Jan 3, 2022

Uh oh!

BraisGabin commented Jan 3, 2022

Uh oh!

cortinico commented Jan 5, 2022

Uh oh!

cortinico commented Feb 26, 2022

Uh oh!

BraisGabin commented Feb 28, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

BraisGabin commented Nov 20, 2021 •

edited

Loading

codecov Bot commented Dec 1, 2021 •

edited

Loading

BraisGabin Dec 2, 2021 •

edited

Loading