dart-lang / linter

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

proposal: `no_mutually_exclusive_parameters` #4805

Open Fernandomr88 opened 7 months ago

Fernandomr88 commented 7 months ago

Description

Whenever you're implementing a Container for example and you use both color and decoration at the same time, there's no error shown but an exception is thrown only when the widget is built in runtime, not even on compilation. Code example:

Container(
  height: 150,
  width: 150,
  color: Colors.white, ///<- color
  decoration: BoxDecoration(borderRadius: BorderRadius.circular(100), color: Colors.white), //<- color again
  child: Image.asset(
    'assets/img/X.gif',
    height: 100,
  ),
));

Details

Cannot provide both a color and a decoration. To provide both, use decoration: BoxDecoration(color: color)

Kind

Guard against runtime errors

Bad Examples

Container(
  height: 150,
  width: 150,
  color: Colors.white, ///<- color
  decoration: BoxDecoration(borderRadius: BorderRadius.circular(100), color: Colors.white), //<- color again
  child: Image.asset(
    'assets/img/X.gif',
    height: 100,
  ),
));

Good Examples

Container(
  height: 150,
  width: 150,
  decoration: BoxDecoration(borderRadius: BorderRadius.circular(100), color: Colors.white),
  child: Image.asset(
    'assets/img/X.gif',
    height: 100,
  ),
));

Discussion

This is coming from two previous created issues.

https://github.com/flutter/flutter/issues/133760 https://github.com/flutter/flutter/issues/133456

Discussion checklist

I have come to this error multiple times, only in runtime - not even compile time, when I started to code using Flutter. This could have been prevented by this simple rule.

bwilkerson commented 7 months ago

@goderbauer

goderbauer commented 7 months ago

It is correct that you can't have a color and decoration on a Container at the same time. Right now we can only detect this situation at runtime. A lint for this would be nice to have. However, this is not the only place in the API where we have mutually exclusive arguments. I would prefer it if we had a generic way of marking mutually exclusive arguments (and linting based on that) instead of implementing individual one-off lints for very specific cases.

Fernandomr88 commented 7 months ago

It is correct that you can't have a color and decoration on a Container at the same time. Right now we can only detect this situation at runtime. A lint for this would be nice to have. However, this is not the only place in the API where we have mutually exclusive arguments. I would prefer it if we had a generic way of marking mutually exclusive arguments (and linting based on that) instead of implementing individual one-off lints for very specific cases.

That would be much better indeed. Should I change the proposal?

bwilkerson commented 7 months ago

I would prefer it if we had a generic way of marking mutually exclusive arguments ...

The first option that comes to mind (though maybe not the best) would be to define an annotation (like @MutuallyExclusive('group name'), only better) and allow the analyzer to warn if

A language feature might be nicer, but I don't know whether we could get that. (Also the annotation might be more flexible because any style of parameter could be in the same group.)

Fernandomr88 commented 7 months ago

A language feature might be nicer, but I don't know whether we could get that. (Also the annotation might be more flexible because any style of parameter could be in the same group.)

Agreed, should I try and propose this on dart-lang/language? I have tried on flutter and was instructed to come here (as mentioned issues above)

bwilkerson commented 7 months ago

If we define an annotation then we'll implement, warning rather than lints, and we can move this issue to the 'sdk' repository. But for now this is a reasonable place for the issue.

pq commented 7 months ago

💯

I captured some related thoughts in https://github.com/dart-lang/sdk/issues/47712.

Input welcome!

Fernandomr88 commented 7 months ago

💯

I captured some related thoughts in dart-lang/sdk#47712.

Input welcome!

Wow, awesome to see that this is already a work in progress. Thanks @pq

pq commented 7 months ago

Sure thing. To be clear though, it's just a proposal... Please add any context there. Any additional motivation is greatly valued!