Linter conflict resolution

[azjezz]

copying from #287 (comment)

Also, it would be nice to have to ability to disable a linter within a linter.

e.g: CamelCaseEnumMemberssLinter::getConflicts returns a vector of linters classes that should be disabled ( e.g shout case enum members linter )

to explain further, the idea is to add a new Linter::getConflicts(): vec<classname<Linter >> method, this method will return a list of linters that should be disabled if the current linter is enabled.

example:

assume company/company-hack-linters package provides Company\Linter\CamelCaseEnumMemberssLinter::class linter.

This linter ensures that all enum members are using CamelCase ( such as Permission::Read instead of Permission:READ ), this linter would be conflicting with the hhast builtin linter Facebook\HHAST\ShoutCaseEnumMembersLinter::class ( it's getConficlits() method returns vec[Facebook\HHAST\ShoutCaseEnumMembersLinter::class] ).

If #423 is implemented, there's two ways to configure linters ( can be discussed in there ).

1. builtin linters are always enabled
2. builtin linters are configured within the generated file.

If we go with option (2), it would make sense for hhast to throw an exception, if a linter conflicts with another linter in the list, the end user would than have to manually remove Facebook\HHAST\ShoutCaseEnumMembersLinter::class from their configuration.

If we go with option (1), the better approach would be for hhast to resolve the conflict automatically, but also log the linters that were disabled through this process in stderr.

e.g:

$ ./tools/hhast
[conflict-resolver]: Linter `Facebook\HHAST\ShoutCaseEnumMembersLinter` has been disabled since it conflicts with `Company\Linter\CamelCaseEnumMemberssLinter`

how conflict resolution happens, which linter takes priority when multiple linters conflict with each other, is up for discussion.

[fredemmott]

If we go with option (2), it would make sense for hhast to throw an exception, if a linter conflicts with another linter in the list, the end user would than have to manually remove Facebook\HHAST\ShoutCaseEnumMembersLinter::class from their configuration.

There's two other options that might make sense:

If I do vec[builtin_linters, linter_that_conflicts], I clearly want the linter that conflicts.

We could track this with a priority system, which could either be explicit in the linters, or bundle-based, e.g.

enum PRIORITIES: int {
  VIA_BUILTIN_BUNDLE = 0,
  VIA_THIRD_PARTY_BUNDLE = 1,
  SPECIFIC_LINTER_REQUESTED = 2
};

... or, if we take a vec<Bundle|Linter> , we could just go left-to-right.

There would still be the potential for unresolvable conflicts via this system.

[fredemmott]

Warnings vs errors

I think we should always do hard errors - having a warning on every run IMO shouldn't be considered acceptable.

Warn-and-continue indicates that:

• we're not confident that the default behavior we've selected is reasonable
• we think it's important enough to tell the user every time we've ran it

IMO conflict resolution should end in 'this is definitely the right thing to do', or "I don't know", and the best thing to do for "I don't know" is hard-fail.

[fredemmott]

Manual resolution

a.k.a. warning/error suppression:

Perhaps some_bundle()->without(SomeSpecificLinter::class) ?