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

Define style guide as data interchange format #57155

Closed
seaneagan opened this issue Feb 5, 2015 · 8 comments
Closed

Define style guide as data interchange format #57155

seaneagan opened this issue Feb 5, 2015 · 8 comments
Labels
analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. type-enhancement A request for a change that isn't a bug

Comments

@seaneagan
Copy link

Currently the style guide is just a markdown doc. So there is no metadata about the style guide rules for the linter to consume, so it will have to define its own. It would be much nicer if the lints produced by the linter were associated back to a style guide rule via a rule id. The rule metadata could then be looked up in the style guide by that id. Would probably be either JSON or YAML. That would allow:

  • maintaining rule metadata in a single place
    • id
    • type e.g. DO/DONT/PREFER/AVOID
    • title the text after the type
    • description
    • good and bad examples
  • Contextual view of style guide rule violations for lints in editors (e.g. in tooltips and problems view) which could also link to the appropriate section of the actual style guide.
  • Filtering of rules by their type which could be useful for Linter Configurability #57153.
  • Generating the actual style guide markdown doc.

I filed the issue here instead of at https://github.com/dart-lang/www.dartlang.org since this seems like a logical place for the discussion, but I can move it there if desired.

@pq @munificent thoughts?

@sethladd
Copy link
Contributor

sethladd commented Feb 5, 2015

unit testable style guides. Love it! :)

@pq
Copy link
Member

pq commented Feb 5, 2015

So many good ideas here!

+1000 across the board.

Specifically, to @sethladd 's point, the good and bad examples could easily be translated into test data which would be cool. When I did the first cut of the formatter I did just this only manually. I'm not sure how flexible markdown is to be honest... If we wanted to embed metadata maybe we'd need to look into MMD or some other MD superset?

More generally, I think tying the style guide and linter rules and tool output would be great. Defining it in one place would be a win for all.

@seaneagan
Copy link
Author

Interesting, so for those rules in the style guide that are actually lintable (violations are machine detectable), the bad example could be used as the text case against a lint being produced, and the good example could be used as the expected output when testing a quick fix corresponding to that rule.

Looks like the current style guide has the good and bad examples in separate dart files. That seems nice because you would get syntax highlighting in editors, could run the analyzer on them, etc. Alternatively could use the same format as dart_style does.

One tricky thing will be defining the rule ids. Naming conventions will need to be defined. I think it is best not to include DO/DONT/PREFER/AVOID in the rule id. That way you can change the type without changing the rule id. Maybe just a no prefix for the DONT/AVOID rules. Also, maybe the rules could be namespaced by what group they are in e.g. whitespace.noUseTabs.

Another question: Would it make sense to move the style guide to its own repo to ease consumption from this repo, the dartlang.org repo, and potentially dart_style? Maybe the style_guide package could expose the style guide metadata described aboe as assets, and it could have a script in its tool directory to generate the actual style guide markdown doc, which could then be uploaded to the dartlang.org repo.

@pq
Copy link
Member

pq commented Feb 7, 2015

Good points @seaneagan. Rule ids and naming conventions are tricky. I did a little surveying and it seems like there's little consensus in related tools.

As for moving the style guide and building a package around it, that's an interesting idea. I'd be curious how @munificent and @sethladd weigh in. In the meantime, looking at the markup, it appears to be using a superset of MD so that's cool as far as potentially embedding metadata.

Regardless of how we achieve it, I think having a nice linking here would be awesome. Maybe crazy but for lints associated with style guide entries we could even emit (in some kind of uber verbose mode) URLs for added rationale...

@sethladd
Copy link
Contributor

sethladd commented Feb 7, 2015

As long as it's easy to generate a markdown file that contains all the rules and the text, I'm flexible on where the canonical version of the style guide lives.

@pq pq added the type-enhancement A request for a change that isn't a bug label Feb 10, 2015
@pq
Copy link
Member

pq commented Feb 10, 2015

FWIW I did some experimenting with generating rule docs. In the long run, I'd love to share definitions with the style guide. In the meantime, the process is a little manual at first (you need to harvest the text from the guide), but once it's done, the docs get auto generated.

For example:

Lots of details to hammer out but as a jumping off point not too bad.

Needless to say, comments/feedback most welcome!

@seaneagan
Copy link
Author

Way cool!

I guess a next step would be pulling out the description, details, kind, etc. into a separate yaml file (for starters, could switch to markdown embedding later) since that will allow reading/writing the rules without touching any actual dart files. That may bump up against http://dartbug.com/21020, but there are workarounds such as a transformer which produces a dart file with a string variable with the yaml file's contents (or just start with that).

Could also potentially parse the "kind" (DO, DONT, etc.) from the description (or keep it separate and concatenate).

@pq
Copy link
Member

pq commented Mar 14, 2017

I love this idea (still) but since it's not on any short-term roadmaps, closing out for now. 😢

@pq pq closed this as completed Mar 14, 2017
@devoncarew devoncarew added analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. labels Nov 18, 2024
@devoncarew devoncarew transferred this issue from dart-lang/linter Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants