[no-conflicting-lifecycle] false positives

[rafaelss95]

Description and reproduction of the issue
As mentioned in mgechev/codelyzer#560 (comment), mgechev/codelyzer#588 and DoCheck#description:

Typically, you should not use both DoCheck and OnChanges to respond to changes on the same input.

This rule should report only if a variable is being checked in both ngDoCheck and ngOnChanges methods, not as it currently does that reports if you implement both DoCheck and OnChanges or have corresponding methods.

Versions

package	version
@angular-eslint	12.0.0
TypeScript	4.3.0
ESLint	7.27.0

# Please run `npx ng version` in your project and paste the full output here:
Angular CLI: 12.0.0
Node: 14.16.0
Package Manager: npm 6.14.11
OS: darwin x64

Angular: 12.0.0
... animations, cdk, cli, common, compiler, compiler-cli, core
... forms, language-service, localize, material
... platform-browser, platform-browser-dynamic, platform-server
... router, service-worker

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1200.0
@angular-devkit/build-angular   12.0.0
@angular-devkit/core            11.2.11
@angular-devkit/schematics      11.2.11
@nguniversal/builders           12.0.0
@nguniversal/common             12.0.0
@nguniversal/express-engine     12.0.0
@schematics/angular             12.0.0
ng-packagr                      12.0.0
rxjs                            6.5.5
typescript                      4.2.4

• I have tried restarting my IDE and the issue persists.
• I have updated to the latest supported version of the packages and checked my ng version output per the instructions given here.

[rafaelss95]

Being quite honest: this is a very difficult (not to say impossible) rule to implement it correctly, due to the many possibilities.

What actually defines whether an input is being checked in both methods at all? Should the code below trigger the rule? If so, why?

@Component()
class ChildComponent implements DoCheck, OnChanges {
  @Input() person!: Person;
  readonly differ = this.keyValueDiffers.find({}).create();
  lastId?: string;

  constructor(private readonly keyValueDiffers: KeyValueDiffers) {}

  ngDoCheck() {
    const changes = this.differ.diff(this.person);

    if (!changes) {
      return;
    }

    changes.forEachChangedItem(({ currentValue }) =>
      console.log('changed: ', currentValue),
    );
    changes.forEachAddedItem(({ currentValue }) =>
      console.log('added: ', currentValue),
    );
    changes.forEachRemovedItem(({ currentValue }) =>
      console.log('removed: ', currentValue),
    );
  }

  ngOnChanges() {
    this.lastId = this.person.id;
  }
}

[JamesHenry]

Apologies folks, because of personal life pressures I have had to limit angular-eslint to v13 of Angular and v8 of ESLint, so this breaking change requirement will be bumped to v14

[devoto13]

IMO this rule does not make any sense and should be removed together with the confusing statement in the Angular docs:

When the default change detector detects changes, it invokes ngOnChanges() if supplied, regardless of whether you perform additional change detection. Typically, you should not use both DoCheck and OnChanges to respond to changes on the same input.

Typically, nobody uses ngDoCheck to respond to input changes as there is ngOnChanges for it. Typically, one should not use ngDoCheck at all unless they know exactly what they are doing.

[JamesHenry]

The initial decision, and breaking change, I am going to make on this one, is that in v15 this rule will no longer be part of the recommended config exported by @angular-eslint/eslint-plugin.

This is in light of the various comments above concluding that it is not universally applicable and not necessarily doing a good job of fulfilling its intended purpose.

As @rafaelss95 points out, its intended purpose may well be very difficult to actually implement consistently, and we may have to consider full deprecation of this rule in future.