dart-lang / linter

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

avoid_setters_without_getters false positive for Angular components #1788

Open davidmorgan opened 4 years ago

davidmorgan commented 4 years ago

https://dart-lang.github.io/linter/lints/avoid_setters_without_getters.html corresponds to https://dart.dev/guides/language/effective-dart/design#dont-define-a-setter-without-a-corresponding-getter, except that the Effective Dart rule has an exception for Angular components.

We could avoid those false positives by allowing setters without getters when there is an annotation Component.

What do you think, please?

pq commented 4 years ago

Sounds reasonable to me.

@MichaelRFairhurst : any thoughts?

MichaelRFairhurst commented 4 years ago

I'd recommend looking for an @Input on the setter itself over an @Component (or don't forget @Directive) on the class.

However, @Input annotations can be inherited, so if you don't want those false positives, and don't want to look up the inheritance tree, then @Component/@Directive is probably best.

pq commented 4 years ago

Thanks! Could you give me a concrete test case? That'd help a lot. 👍

davidmorgan commented 4 years ago

@matanlurey what do you think, please? Proposal is to make avoid_setters_without_getters ignore Angular components, and the discussion is the best way to identify those Angular setters :)

MichaelRFairhurst commented 4 years ago

Some test cases:

import "package:angular/angular.dart";

@Component(selector: "concrete-comp", template: "")
class ConcreteComponent extends AbstractComponent {
  @Input()
  set classInput(int v) {}
}

@Directive(selector: "[directive]")
class ConcreteDirective extends AbstractDirective {
  @Input()
  set classInput(int v) {}
}

class AbstractComponent {
  @Input()
  set inheritedInput(int v) {}
}

class AbstractDirective {
  @Input()
  set inheritedInput(int v) {}
}

There are also some negative cases I could draw here (positive cases? where the lint should still flag).

import "package:angular/angular.dart";

@Component(selector: "concrete-comp", template: "")
class ConcreteComponent extends AbstractComponent {
  // not an @Input, could/should be flagged
  set classInput(int v) {}
}

@Directive(selector: "[directive]")
class ConcreteDirective extends AbstractDirective {
  // not an @Input, could/should be flagged
  set classInput(int v) {}
}

class AbstractComponent {
  @Input()
  set inheritedInput(int v) {}

  // not an @Input, could/should be flagged
  set rogueSetter(int v) {}
}

class AbstractDirective {
  @Input()
  set inheritedInput(int v) {}

  // not an @Input, could/should be flagged
  set rogueSetter(int v) {}
}

Thinking more on it, there are other fields than @Input to consider too. Like @ContentChild, @ContentChildren, @ViewChild, @ViewChildren. Same concepts apply.