New lint: avoid_compile_constants

[tomlenehan-google]

Avoid using compile time constants

The lint would trigger upon the use of the three Flutter fromEnvironment constructors:
int.fromEnvironment
string.fromEnvironment
bool.fromEnvironment

bool.hasEnvironment should also trigger the lint.

Using a compile constant creates a hidden global state in the program, with no defined method to determine the state or understand its use when reading the code (unlike flags a compile time constant might only be referenced during the fromEnvironment constructor call). If used heavily it makes the program hard to understand.

[pq]

LGTM. I think we should tweak the name (favoring do_not_use over avoid) and being explicit about the APIs we're restricting.

After chatting w/ @bwilkerson, I'm in favor of:

do_not_use_fromEnvironment.

[pq]

Hmm, actually, since hasEnvironment is in there too, it might be better to say do_not_use_environment.

[bwilkerson]

Agreed.

[rakudrama]

I have to say I don't really understand the motivation. The environment is baked into the app by the compiler. You can't do anything in the app to change the result, so it is more lake a configuration rather than (mutable) state.

One big advantage of, e.g. const bool.fromEnvironment is that since it is baked into the app by the compiler, the app can be optimized against the configuration, for example, tree-shaking all the dependent code.

This is not true of an alternative like a command-line option, which must not be optimized away in case the app is passed a command line with the option. Avoiding the 'environment' will always result in larger compiled apps.

[pq]

The environment is baked into the app by the compiler.

👍

I think this is for the case where you specifically don't want that.

Maybe @tomlenehan-google can better motivate and I can update the docs to capture when this might be most appropriate?

Thanks for the feedback!