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

📃 add detekt-generator module to generate documentation and default config #563

Merged
merged 20 commits into from
Nov 28, 2017

Conversation

Mauin
Copy link
Collaborator

@Mauin Mauin commented Nov 19, 2017

Resolves #343

This PR:

  • Creates a new module detekt-generator which is a new CLI module that compiles sources from the detekt-rules module and scans them for relevant content to be put into the documentation.
  • Multiple collectors collect data on RuleSetProviders, Rules and MultiRules to compose a list of all necessary data to print the documentation and the default-content.
  • Printers then take this data and print a *.md file for the documentation of each Rule Set Provider and it's rules and their configuration options.
  • Also a default-detekt-config.yml is generated.
  • I touched a couple of rules and added the necessary KDoc for them to get at least some documentation generated.

Currently I still included the generated documentation and the default config. But they're mostly useless at the moment as we only have very few rules documented in the correct style.

Next steps would be adding the necessary KDoc so that we get a full documentation generated from the script.

Currently missing: Tests (working on it, but wanted to share this first for feedback)

Copy link
Member

@schalkms schalkms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good work! :)
I like it.
I know this is just the first prototype. However, I think for generating the markdown content detekt should use an internal DSL. Furthermore, I think it would be a good idea to decouple the generation process. This means detekt could easily provide generators producing other formats.

@Mauin
Copy link
Collaborator Author

Mauin commented Nov 20, 2017

@schalkms Yep, you're totally correct that the printing part could be a bit nicer. For this first version I mostly focussed on gathering all the necessary data with the collectors.
After the collection process we already have a list of RuleSetPage which we could use to also generate other formats. The necessary data should already be there. We would only need some different printers.

@arturbosch
Copy link
Member

Yep, I like it! For an indepth review I need more time but looks fine so far! I like the rule template which we can mention in contributing.md so every new rule must confirm to it. An internal dsl sounds also good. More to learn and play with :).

Copy link
Member

@schalkms schalkms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@Mauin
Copy link
Collaborator Author

Mauin commented Nov 25, 2017

Gonna add a small yaml DSL as well to clean up the config printer, then some tests and then this is good to go 👍

@schalkms
Copy link
Member

👍

@@ -3,7 +3,7 @@ package io.gitlab.arturbosch.detekt.generator.out
/**
* @author Marvin Ramin
*/
class Markdown(var content: String = "") {
sealed class MD(open var content: String = "") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we find a better name here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about naming the sealed class Markdown and then have MarkdownContent and MarkdownList that extend Markdown?

Copy link
Member

@schalkms schalkms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! I like it. This is no more WIP?

@Mauin
Copy link
Collaborator Author

Mauin commented Nov 26, 2017

@schalkms I think this is good to go for now. We can still improve it later on.
Only question is where to put the generate documentation and the default detekt config this generates for now.
Next steps are getting all the KDoc ready so that we get rid of all the TODOs in the documentation 🙃

@schalkms
Copy link
Member

Right. I agree.

}

private fun printRule(rule: Rule): String {
return markdown {
Copy link
Member

@arturbosch arturbosch Nov 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we refactor all such functions to expression syntax? -> less indentation + looks better
Or is it just me?

@ all printers

@arturbosch
Copy link
Member

arturbosch commented Nov 27, 2017

Awesome work looks good with all the dls and stuff :D
Will check it out locally too ^^
Edit: Jep works great.

@arturbosch
Copy link
Member

One thing: The rule sources path is hardcoded to private const val RULES_SOURCES_PATH = "./detekt-rules/src/main/kotlin". This makes it only possible to run the generator from the project path and not the generator sub project. Should this be configurable? -> You've created an Args object which for now has only the help parameter?

@Mauin
Copy link
Collaborator Author

Mauin commented Nov 27, 2017

Yeah, I hardcoded it because it only really makes sense at the moment to run the generator over the rules module.
We can make it a CLI arg (the Args class is mostly a leftover from when I copy pasted the -cli module) but I don't think the generator module needs to be super configurable as it's mostly an internal utility.

@arturbosch
Copy link
Member

Yep, then lets delete the Args class + add a require and check if we are in the top project and detekt-rules exist?

@Mauin
Copy link
Collaborator Author

Mauin commented Nov 27, 2017

@arturbosch Sounds good. Will do that.
WDYT about the directory where the generated files should live? Currently I just threw them into the generator module for debugging. But I guess they should live somewhere else.

@arturbosch
Copy link
Member

I think it is ok for now. Well we could move it to top level so all github repo visitors see a documentation folder and can take a look at it. Later we will integrate the doc files into jekyll/jbake or something.

@arturbosch arturbosch added this to the RC6 milestone Nov 27, 2017
@arturbosch arturbosch merged commit df2e0f7 into detekt:master Nov 28, 2017
@Mauin Mauin mentioned this pull request Dec 3, 2017
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants