Skip to content

fix: exclude decorators from GetFunctionHeadLoc reported range#772

Merged
fansenze merged 2 commits intomainfrom
fix/explicit-function-return-type-decorator-loc-20260422
Apr 22, 2026
Merged

fix: exclude decorators from GetFunctionHeadLoc reported range#772
fansenze merged 2 commits intomainfrom
fix/explicit-function-return-type-decorator-loc-20260422

Conversation

@fansenze
Copy link
Copy Markdown
Contributor

@fansenze fansenze commented Apr 22, 2026

Summary

GetFunctionHeadLoc is supposed to mirror typescript-eslint's
getFunctionHeadLoc, which explicitly skips leading decorators on
MethodDefinition / PropertyDefinition when computing the reported
head range. The old implementation used node.Pos() / TrimNodeTextRange
directly, and in tsgo that sits before the first decorator — so any
rule using this helper (explicit-function-return-type, require-yield,
accessor-pairs, no-misused-promises) reported the decorator line/column
instead of the method/property itself.

Concretely, for

```ts
class HookLifeCycle {
@LifecycleHook()
async didLoad() { ... }
}
```

ESLint reports line 4 col 3 (at async); rslint reported line 3 col 3
(at @LifecycleHook).

Fix: new nodeStartSkippingDecorators helper walks node.Modifiers()
to find the last decorator and starts the range at the next token.
It is plumbed through every branch of GetFunctionHeadLoc where the
"start" came from the decorated node or its property parent.

Related Links

n/a

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

Align GetFunctionHeadLoc with typescript-eslint's getFunctionHeadLoc,
which skips leading decorators on MethodDefinition / PropertyDefinition
when computing the head range. The previous implementation used
node.Pos() directly, which in tsgo sits before the first decorator,
so rules like explicit-function-return-type reported the decorator
line/column instead of the method itself.

Added a shared nodeStartSkippingDecorators helper and threaded it
through the method / getter / setter / constructor branches and the
property-initializer branches for arrow and function expressions.
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the GetFunctionHeadLoc utility to exclude leading decorators from the reported range of class members, aligning the behavior with typescript-eslint. A new helper function nodeStartSkippingDecorators was introduced, and existing tests for require_yield and explicit_function_return_type were updated to reflect the adjusted reporting locations. A review comment suggests initializing the search position for function parameters from the start of the node after decorators to avoid incorrectly matching parentheses within decorator factories.

Comment thread internal/utils/ts_eslint.go Outdated
Address Gemini Code Assist review: when the member is a nameless
Constructor and it carries a decorator factory (e.g. @dec()), using
node.Pos() as the parameter-paren search origin matches the `(` of
the decorator factory first and produces an inverted range.

Anchor the scan at the first token after the decorators instead,
keeping the prior behaviour for named methods/getters/setters via the
name.End() override. Adds a utils-level regression test that asserts
the range spans just the `constructor` keyword.

Also sync the `require-yield` JS mirror test (and its snapshot) with
the Go expectations updated in the previous commit — the columns now
land after the decorators instead of on `@`.
@fansenze fansenze force-pushed the fix/explicit-function-return-type-decorator-loc-20260422 branch from 3f0bda5 to f697667 Compare April 22, 2026 07:06
@fansenze fansenze merged commit 5e7e4c8 into main Apr 22, 2026
9 checks passed
@fansenze fansenze deleted the fix/explicit-function-return-type-decorator-loc-20260422 branch April 22, 2026 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants