Honour disable comments in FIELD_NUMBERS_ORDER_ASCENDING

[Eljees]

Fixes #558

The problem

// protolint:disable:this FIELD_NUMBERS_ORDER_ASCENDING on a field line is
silently ignored:

$ cat .protolint.yaml
lint:
  rules:
    add:
      - FIELD_NUMBERS_ORDER_ASCENDING

$ cat demo.proto
syntax = "proto3";
message M {
  uint64 a = 2; // protolint:disable:this FIELD_NUMBERS_ORDER_ASCENDING
  uint64 b = 1; // protolint:disable:this FIELD_NUMBERS_ORDER_ASCENDING
}

$ protolint lint demo.proto
[demo.proto:4:3] field a should be after b (ascending order expected)

The same is true for the block form inside a message, so at the moment there is no
way to turn this rule off for part of a message.

That the directive itself is fine is easy to show — on the same line, for another
rule, it works:

$ cat other.proto
syntax = "proto3";
message M {
  uint64 badName = 2; // protolint:disable:this FIELD_NAMES_LOWER_SNAKE_CASE
  uint64 alsoBad = 1;
}

$ protolint lint other.proto
[other.proto:4:3] Field name "alsoBad" must be underscore_separated_names like "also_bad"
[other.proto:4:3] field badName should be after alsoBad (ascending order expected)

badName is exempted from FIELD_NAMES_LOWER_SNAKE_CASE as asked, while
FIELD_NUMBERS_ORDER_ASCENDING ignores the directive on the very same line.

The cause

extendedDisableRuleVisitor is what interprets the directives, and it does so per
element: VisitField reads f.Comments / f.InlineComment and, when a directive
matches, does not call the inner rule at all.

FieldNumbersOrderAscendingRule does not implement VisitField or VisitEnumField
— it walks message.MessageBody and enum.EnumBody itself inside VisitMessage /
VisitEnum. The wrapper therefore never reaches the fields, and nobody reads the
comments attached to them.

The change

Interpret the directives in the rule itself, with the same
disablerule.Interpreter the wrapper uses.

One decision worth calling out: a disabled field still takes part in the ordering
chain, only the report about it is suppressed. Dropping disabled fields from the
chain also makes the reported case quiet, but then the next field is compared
against nothing and effectively stops being checked — the opposite of what the
issue asks for ("when my colleagues add uint64 c = 3 I want protolint to force it
in the end of the message"). With this patch:

# a=2, b=1 both disabled, then c appended in order -> quiet
$ protolint lint wish.proto ; echo $?
0

# same two, then c out of order -> still reported
$ protolint lint wish_bad.proto
[wish_bad.proto:5:3] fields b and c have the same number 1

If you would rather have disabled fields dropped from the chain entirely, that is a
two-line change and I am happy to switch.

Tests

New table in fieldNumbersOrderAscendingRule_disable_test.go: fields with the
directive, enum fields with the directive, a field following disabled ones (still
checked), and a field without the directive (still checked).

• before the change: three of the four cases fail, e.g.
  got [field a should be after b (ascending order expected)], but want []
• after the change: all four pass, and the existing 18 cases of
  TestFieldNumbersOrderAscendingRule_Apply keep passing
• go test -race ./...: no new failures. internal/linter/report/reporters
  (TestEnvMatcherReporterFromUnallowedTemplateFile_Report) already fails on a
  clean master here and is unrelated to this change.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1d0e506830

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Share disable state with the visitor wrapper

When a persistent disable is already active upon entering a message or enum, extendedDisableRuleVisitor skips this method, so this newly created interpreter never sees an enable attached to a field and checks cannot resume for later fields in that body. Conversely, if this local interpreter encounters a disable after an earlier ordering failure, return !hasError prevents the wrapper from traversing the children, so the disable is not carried to later declarations. Both cases violate the documented disable-until-enable-or-EOF behavior; the ordering logic needs to participate in the wrapper's single stateful traversal rather than maintaining an independent interpreter.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Process directives on every body element

Because Interpret is called only after the element has been asserted to *parser.Field, persistent directives attached to an option, reserved declaration, nested declaration, or standalone comment are ignored while this method evaluates the fields. For example, option deprecated = true; // protolint:disable FIELD_NUMBERS_ORDER_ASCENDING followed by descending fields still produces a failure; the wrapper visits the option only after VisitMessage has already performed the ordering check. The enum loop has the same problem, so directives from every body element need to be consumed in source order.

Useful? React with 👍 / 👎.