Sdk: dart format and dart analyze inconsistencies

[mit-mit]

I'd expect dart format and dart analyze to be structurally similar, but notice a few inconsistencies:

• One has [directory] as a listed argument, the other doesn't

$ dart help format
Format Dart source code.

Usage: dart format [arguments]

$ dart help analyze
Analyze the project's Dart code.

Usage: dart analyze [arguments] [<directory>]

• I can run dart analyze to analyze CWD; would like to be able to run dart format to format CWD

[bwilkerson]

Sounds like it would be a good idea to have a design doc that spells out some general design principles in order to help keep the design consistent over time. I'd couch this one as:

Verbs that can be applied to the contents of a directory should allow zero or more directories to be specified, defaulting to the current working directory when no directories are provided.

[mit-mit]

@jwren are you looking at this?

[jwren]

Assigned to @munificent, who owns the dart format UX

[jwren]

The solution here is to either:

• have dart analyze not implicitly use the CWD as context, and require dart analyze .
• have dart format use the the CWD as context

After https://dart-review.googlesource.com/c/sdk/+/153900 lands, both with have their appropriate documentation (analyzer to use the CWD, the and the format not using it)

@mit-mit this is a small change, which would you like?

[mit-mit]

My preference is to align with similar commands in flutter

[jwren]

• flutter analyze analyzes the code in the flutter sdk that you have downloaded... thus it does not do either. I am optimistic that with flutter analyze deferring to dart analyze, we can remove this surprise when the user types flutter analyze in some directory
• flutter format works as it does here, requiring a directory or . for the CWD.

This would imply, I suppose, that you'd be in favor of having dart analyze require the user to type the ., I don't have any strong objection (but think that implicit CWD is better). @bwilkerson @munificent do either of you have an opinion here?

[bwilkerson]

I think there's an argument to be made that keeping dart analyze consistent with dartanalyze will ease adoption, but I don't know how big a factor that is. Personally I like having commands default to the cwd, but there might be good reasons not to support it. It _is_ a feature that could easily be added later, while it would be harder to remove.

[mit-mit]

flutter analyze analyzes the code in the flutter sdk that you have downloaded.

That is incorrect, it analyzes the developers app/package (aka the "project"):

$ flutter help analyze
Analyze the project's Dart code.

I think there's an argument to be made that keeping dart analyze consistent with dartanalyze will ease adoption

Interesting, I think of this in the opposite way: This is our golden change to change the current experience. I hugely in favor of defaulting to CWD as it's what any Flutter developer has been trained to expect.

In summary: it's my strong preference that both dart analyze and dart format take an optional directory argument, and that they just run against CWD when run without a path.

[bwilkerson]

Interesting, I think of this in the opposite way:

Actually, I think we're thinking of this in exactly the same way. :-)

This is our golden change to change the current experience.

I think you're saying that we shouldn't be tied to the past but should understand what our users want and deliver it, which I agree with.

I hugely in favor of defaulting to CWD as it's what any Flutter developer has been trained to expect.

I think you're saying that it's important to match user expectations, which are sometimes rooted in the past, which I also agree with.

In summary: it's my strong preference that both dart analyze and dart format take an optional directory argument, and that they just run against CWD when run without a path.

Great! That's what dartanalyze does, so that's exactly what I was suggesting ought to happen based on user expectations based on past experience.

[munificent]

In summary: it's my strong preference that both dart analyze and dart format take an optional directory argument, and that they just run against CWD when run without a path.

Great! That's what dartanalyze does, so that's exactly what I was suggesting ought to happen based on user expectations based on past experience.

There is a difference here. If dart analyze uses the CWD when the user simply forgot to put an argument in there, they may get some unexpected analysis output, but no harm's done. If the user meant to write dart format foo.dart and forgot the last argument it will potentially irrevocably overwrite many files all throughout the CWD's file tree, arbitrarily deeply. Imagine accidentally typing dart format in the root of your hard disc.

I'm a little hesitant to do that without at least some signal that recursively formatting the entire CWD is what the user wants.

[bwilkerson]

I think that's a valid argument for why the two verbs should be different. It sounds like a good principle is:

If the verb cannot have side effects (such as analyze) then the argument for what to operate on should default to the CWD, but if the verb can have side effects (such as format, run, or fix) then the argument for what to operate on should be required.

(And I'll suggest again that if they aren't already the design principles should be captured somewhere to help guide the future evolution of this interface.)

[munificent]

If the verb cannot have side effects (such as analyze) then the argument for what to operate on should default to the CWD, but if the verb can have side effects (such as format, run, or fix) then the argument for what to operate on should be required.

I like this guideline.

(And I'll suggest again that if they aren't already the design principles should be captured somewhere to help guide the future evolution of this interface.)

Yes, yes, yes. Are you volunteering to write that? ;)

[bwilkerson]

I wasn't, but I'm willing to do so if nobody else will.

[devoncarew]

OK, I think what we've decided on is that:

• it's ok for dart analyze and dart format to have slightly different behaviors (given that analyze doesn't make changes to the user's disk and format does)
• dart analyze should take a directory as an argument and default to the cwd if none is given
• dart format should take a directory as an argument but not default to one if non is provided
• we'd like a style guide :)

Reviewing the current behavior of dart analyze and dart format, I think we're good. It's pretty hard to see the error message from dart format when you don't provide a directory (Missing paths to code to format.), as we then print ~50 other lines of help text. I filed https://github.com/dart-lang/dart_style/issues/938 for that issue - reducing the amount help text we print for dart format by default.

I'll open an issue to track creating a style guide for dartdev. For convince, I'd suggest we just have it as a markdown file in pkg/dartdev.

[devoncarew]

https://github.com/dart-lang/sdk/issues/42864 for the style guide