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

Check if it is feasible to integrate ktlint as a rule set #38

Closed
arturbosch opened this issue May 16, 2017 · 16 comments
Closed

Check if it is feasible to integrate ktlint as a rule set #38

arturbosch opened this issue May 16, 2017 · 16 comments

Comments

@arturbosch
Copy link
Member

pro:

  • get fixes asap from ktlint
  • more rules?

neg:

  • different kotlinc versions
  • two engines -> same work twice? -> shortcuts? invoke ktlint
  • detekt additional formatting rules split
@arturbosch
Copy link
Member Author

ktlint-standard-ruleset is not published to maven central as standalone artifact. Formatting rules were fixed and cleaned up for M11.

@shyiko
Copy link

shyiko commented May 29, 2017

@arturbosch
Copy link
Member Author

@shyiko nice thx. Did not find it on my own :)

@arturbosch arturbosch reopened this May 30, 2017
@arturbosch
Copy link
Member Author

KtLint uses the embeddable kotlin compiler, detekt not.
The new recommend way for formatting is to use the detektIdeaFormat task starting from M11.

@arturbosch
Copy link
Member Author

arturbosch commented Jun 7, 2017

Detekt now uses the embeddable compiler. A KtLint rule set is now feasible.

Idea:

  • Create new rule set with wrappers around all KtLint rules
  • Delete spacing, indentation and newline rules from detekt-formatting, just keep the extra rules which feature ast manipulation
  • Users have now the choice between formatting with detektIdeaFormat and detektKtLintFormat

@vanniktech
Copy link
Contributor

So the idea is to get rid of the formatting part in detekt (detekt-formatting) and leave it up to ktlint?

@arturbosch
Copy link
Member Author

Yeah it would be best. I still do not know what to do with the formatting rules which ktlint does not have. Move them to StyleGuideProvider? Should detekt-rules also support autoCorrect? Should we leave detekt-formatting with all rules and integrate KtLint into that module? Create a new detekt-ktlint-module?

I think a hybrid detekt-formatting module would be best (low change costs). We leave all rules as they are and wait until Ktlint releases new rules or fixes. Then we act and wrap one after the other rules. What do you think?

@vanniktech
Copy link
Contributor

vanniktech commented Jul 9, 2017

I'm at the moment using both tools. Ktlint only for formatting and detekt for everything else. I'd assume that also the way to go for the future.

@arturbosch
Copy link
Member Author

detekt/sonar-detekt#20

@arturbosch arturbosch added this to the 1.1.0 milestone Aug 22, 2017
@arturbosch
Copy link
Member Author

PoC: 18be6bc

Fairly easy to integrate.

@schalkms
Copy link
Member

@arturbosch
I was scanning through the issue list. I think this feature is done, isn't it?

@arturbosch
Copy link
Member Author

arturbosch commented May 20, 2018

Yes, its done, some documentation is needed, but that can be done when we integrate the module into our CI, thanks for the reminder.

@dimsuz
Copy link
Contributor

dimsuz commented Aug 2, 2018

I am new to the project (just integrated it) and I'm a bit lost about role of ktlint in detekt.
So documentation would be helpful indeed.

I was researching why does detekt currently have to very similar rules NoWildcardImports and WildcardImport in its default config, so in the sources I discovered that one is coming from ktlint, another is "native"?

Is this ok, or should I report an issue (or I could submit a PR if you suggest me how this should be resolved).

@schalkms
Copy link
Member

schalkms commented Aug 2, 2018

@dimsuz
Detekt provides a wrapper over Ktlint.
There’s actually some documentation regarding the ktlint integration here.
If you have any suggestions for improvement, please feel free to open an issue. We are grateful for PRs regarding detekt's documentation.

@idrisadetunmbi
Copy link

idrisadetunmbi commented May 27, 2019

Not too sure I understand how this ktlint integration works. If I add the ktlint integration as a formatting ruleset, does it mean detekt will report formatting errors, and if I do not add it, it won't report them? Can I possibly run code formatting with detekt once I add the ktlint integration?

@schalkms
Copy link
Member

schalkms commented May 30, 2019

@idrisadetunmbi I don't quite understand your question.
Detekt provides a wrapper over ktlint's rules with the formatting ruleset.
It depends on whether you have the autoCorrect and active setting set to true in the formatting ruleset.

@lock lock bot locked as resolved and limited conversation to collaborators Sep 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants