search

dart-lang / linter

Linter for Dart.
https://dart.dev/tools/linter-rules
BSD 3-Clause "New" or "Revised" License
628 stars 170 forks source link

proposal: `test_block_last` #4901

Open kevmoo opened 7 months ago

kevmoo commented 7 months ago

Description

When providing optional arguments to package:test functions that take functions (like test and group) supply the named arguments before the function block.

Details

When the test/group "body" is large, it's easy to "lose" the named arguments (like skip, onPlatform, etc) below the anonymous function.

Kind

Enforces (un-formalized) style advice that might be generally applicable, but especially with pkg:test functions that take blocks and (optional) named args.

Bad Examples

test(
  'bob',
  () {
    // code
  },
  skip: 'Reason',
);

Good Examples

test(
  'bob',
  skip: 'Reason',
  () {
    // code
  },
);

Discussion

See also https://dart.dev/tools/linter-rules/sort_child_properties_last

One could argue this should be in Effective Dart or Flutter Style Guide, but it's a bit weird since it references an API defined in a package.

Example of a PR that implements this change:

jacob314 commented 7 months ago

Some prior art of similar lints: https://dart.dev/tools/linter-rules/sort_child_properties_last https://github.com/dart-lang/linter/issues/3317

Would be great if we could make this annotation based to allow people to tag the wide range of cases where they would like to force closures as the last parameter rather than hard coding it just for test_blocks.

bwilkerson commented 7 months ago

Would you please fill in the details in the form above. Even with Jacob's comment above I'm not sure of the semantics of the lint rule being proposed.

But I agree that if this is just about making sure that some argument is always last in the argument list, and assuming that it can be a warning, we should add an annotation to indicate that intent.