Possible bug in diagnostics for doNotSubmit

[bwilkerson]

I expected to have a diagnostic produced by the following code, but there is none.

import 'package:meta/meta.dart';

@doNotSubmit
void a() {}

void b() => a();

Should there be one?

[bwilkerson]

@matanlurey

[matanlurey]

Not necessarily a bug but then definitely documentation that's missing; inevitably code marked doNotSubmit might need to get accessed from somewhere, so we don't check for violations in the same library.

I know there is similar permissiveness in other annotations, so if that seems fine I'll send a patch to update the docs.

It also wouldn't be a huge change to enforce same library violations at this point.

[matanlurey]

@srawlins thoughts?

[srawlins]

I like the sentiment that doNotSubmit might need to be used locally. This is similar to how we don't report usage of a @deprecated element from within another @deprecated element.

But that exact shape might not be helpful here. Do you know, specifically for solo in package:test, @matanlurey ?

[matanlurey]

I think that, for say:

void test({@doNotSubmit bool solo = false}) {
  if (solo) { ... }
}

... this should work out of the box (and does today). The question is if this should be flagged:

void test({@doNotSubmit bool solo = false}) {}

// Same library.
void megaTest() {
  test(solo: true);
}

[bwilkerson]

What is the use case for allowing it to be executed outside of another function that's also marked as @doNotSubmit?

If there isn't one, it might be better to be more restrictive in v1 and to relax it when we have a use case. Making a check like this more restrictive can be a huge breaking change, and sometimes impossible.

[matanlurey]

I'm fine with that. Let me take this bug to increase the restrictive nature of the annotation.

[matanlurey]

CL out: https://dart-review.googlesource.com/c/sdk/+/364382.