linter icon indicating copy to clipboard operation
linter copied to clipboard

proposal: `test_block_last`

Open kevmoo opened this issue 1 year ago • 2 comments

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:

  • https://github.com/dart-lang/build/commit/21487cec82a8e7edf330e4868b096d4665c0f604

kevmoo avatar Feb 29 '24 21:02 kevmoo

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.

jacob314 avatar Feb 29 '24 21:02 jacob314

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.

bwilkerson avatar Feb 29 '24 21:02 bwilkerson