Open bwilkerson opened 5 years ago
I look at dartfmt and dartfix as being mostly complementary tools with also probably some overlap. As long as we don't waste a ton of time re-implementing the same fixes twice, I don't think the overlap is hugely problematic. I care more that users get shipped the fix at all than I care which tools ships it.
I also think there are many non-overlapping cases for the tools:
Any fix that requires static analysis isn't and will probably never be a good fit for dartfmt. We don't want dartfmt to do analysis because it's very slow, and we want to be able to format files that contain type errors or aren't part of a well-structured program. Often, formatting is the first step towards fixing up a broken program, so it needs to be accessible even to code containing static errors.
(This does raise an interesting question around the difference between "static", "compile-time", "type", and "syntax" errors. In practice, that seems to mostly be an academic question.)
Any fix that requires human interaction "Should I change X to Y or Z? Please choose." isn't a good fit for dartfmt. Dartfmt is a batch mode, non-interactive tool.
Fixes that are purely syntactic are a good fit for dartfmt. Dartfmt can probably apply them faster than dartfix can, and can apply them even when the code can't be analyzed. Using the formatter sends a clearer signal to users that the change has no semantic affect. Syntax changes typically affect the formatting, so applying them in dartfmt lets you get output that both contains the change and is still correctly formatted.
Pragmatically, people are using and like the fixes dartfmt supports. It's a well-known tool that users trust and know how to run. I don't think it will make users happy to remove these features now in the name of abstract ideals of orthogonality, so we're stuck with them.
I understand your arguments, but I still think it could be confusing to new users who have no opinion where this feature should live. Having a single feature split between two tools just seems wrong to me.
Perhaps a better time to tackle this would be if we move to a single command with sub-commands, such as dart format
and dart fix
.
@mit-mit @kevmoo Curious whether you have opinions.
Will dartfix support fixes that can be applied to code containing static errors? If so, and the performance is good enough, I'd be fine with removing the fixes from dartfmt and putting them only in dartfix if it seems like our users are OK with it.
Yes. At least one of the fixes it already supports depends on looking for a semantic error in order to figure out which code to fix.
It'll be easier for me to explain if there's only one tool that makes these kinds of fixes.
@munificent @danrubel
I think it's great that we added some updating support to
dartfmt
to help our users. However, we have now started working ondartfix
as a tool for automating code changes. I think it's worth having a discussion about what the best path forward is from our users perspective.Should we have a single tool that automates code changes? If so, should that be
dartfmt
ordartfix
, or should we have two tools that apply fixes, withdartfmt
performing syntactic fixes whiledartfix
performs semantic fixes?Personally, I think all of the fixes should be implemented in one tool, but I'm interested to know other people's thoughts.
(This might impact https://github.com/dart-lang/dart_style/issues/753 and https://github.com/dart-lang/linter/issues/1338.)