Repository navigation
Conversation
Check plugins only see the IDs of their own rules, so a plugin cannot tell when one of its rules runs together with a rule it replaces or contradicts, builtin or from another plugin. The new field carries the IDs of every rule the caller runs in the same check. See bufbuild/buf#4722.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
enabled_rule_idstoCheckRequest: the IDs of every rule the caller runs in the same check, across builtin rules and all plugins. Follows up on bufbuild/buf#4722, as suggested there.Why
A plugin only sees its own
rule_ids, so it cannot tell when one of its rules runs together with a rule it replaces or contradicts. buf-lint-extra hasPACKAGE_DIRECTORY_MATCH_EXTRA, which replacesPACKAGE_DIRECTORY_MATCH, andPACKAGE_CAMEL_CASE, which contradictsPACKAGE_LOWER_SNAKE_CASE, among others. WithSTANDARDenabled, every file fails one side until the user copies the rightlint.exceptlist from the README. With this field the plugin can report the clash once and name the rule to except. A plugin can also pick option defaults based on what else runs, which buf-lint-extra already does among its own rules.Semantics
use/exceptresolution. The list is what the caller will actually run, of the same rule type asrule_ids. In Buf, deprecated IDs named in the configuration appear as their replacements. Path-based ignores (ignore,ignore_only, comment ignores) do not remove IDs, since they filter annotations afterwards.lintorbreakingsection with its plugins, or one policy. Buf already resolves and runs each of these as a separate pass, and a plugin running in a policy cannot change the module'sexceptlist anyway.rule_ids. So an empty list means "not provided", and a non-empty list is a superset ofrule_ids.rule_idsis split over several requests, each one carries the full list.Validation
Items have the same constraints as
rule_ids, plusunique. There is nomax_items: a cap would make a large configuration fail validation inside every plugin, and paging makes no sense for a list that every request needs whole.I also wanted a message-level rule for the subset relation:
It compiles, but
buf lintcannot check it in this repository yet. cel-go'spb.Db.RegisterDescriptorswaps a descriptor for the copy linked into the binary when one exists at the same path, and buf linksbuf/plugin/check/v1/check_service.protowithout the new field, soPROTOVALIDATEreportsundefined field 'enabled_rule_ids'. The same file under another path lints cleanly. The rule could be added once buf ships with the regenerated SDK, if you want it.Coupling
The field is informational. Plugins that do not read it are unaffected. A plugin that looks for a specific builtin ID couples itself to that ID, as its documentation already does today when it tells users what to put in
lint.except.Buf side
Buf would fill the field in one place:
multiClient.Checkinprivate/bufpkg/bufcheck/multi_client.goalready holds the full resolved list before narrowing it per client, so lint, breaking, the LSP andprotoc-gen-buf-lintwould all get it. bufplugin-go would expose it asRequest.EnabledRuleIDs(). I can follow up with both once the field is settled.Open questions
enabled_rule_ids, or something else?configured_rule_idsseemed misleading, sinceConfiguredRulesin Buf spans the module and its policies.max_items. Is a cap wanted anyway?