Skip to content

JS-1381 Fix S6598 false positive for interfaces used as defineEmits type argument in Vue#6525

Open
sonar-nigel[bot] wants to merge 7 commits intomasterfrom
fix/JS-1381-fix-fp-on-s6598-interface-used-as-defineemits-type-argument-in-vue-script-setup-sonnet
Open

JS-1381 Fix S6598 false positive for interfaces used as defineEmits type argument in Vue#6525
sonar-nigel[bot] wants to merge 7 commits intomasterfrom
fix/JS-1381-fix-fp-on-s6598-interface-used-as-defineemits-type-argument-in-vue-script-setup-sonnet

Conversation

@sonar-nigel
Copy link
Contributor

@sonar-nigel sonar-nigel bot commented Mar 5, 2026

Fix a false positive in S6598 where interfaces with only call signatures were incorrectly flagged when used as the type argument of defineEmits<T>() in Vue 3 <script setup> blocks. This pattern is idiomatic Vue 3 — each call signature represents a distinct emitted event and cannot be replaced by a function type alias.

Changes

  • Extends decorator.ts with two sequential guards in the interceptReport callback:
    • Guard 1 (isInsideVueSetupScript): passes through all reports outside Vue <script setup> unchanged
    • Guard 2 (isDefineEmitsTypeArg): suppresses reports only when the flagged node is a defineEmits type argument
  • Handles three node shapes: inline type literal, named type alias, and named interface
  • Regular S6598 violations inside <script setup> unrelated to defineEmits continue to be reported
  • Adds tests covering valid cases (interface, inline type literal, named type alias as defineEmits type argument) and the invalid case (interface unrelated to defineEmits inside <script setup>)

Relates to JS-1381.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 5, 2026

Ruling Report

No changes to ruling expected issues in this PR

@sonar-nigel sonar-nigel bot marked this pull request as ready for review March 6, 2026 06:29
@francois-mora-sonarsource
Copy link
Contributor

[4E] Missing upstream FP assertion

The new Vue test cases use rule (already decorated). A useful addition would be a test that imports tsEslintRules['prefer-function-type'] directly (before decoration) and asserts that the upstream rule does raise an issue on the defineEmits FP pattern — for example, an interface with only call signatures used as a defineEmits<T>() type argument in a Vue <script setup> block.

This acts as a sentinel: if the upstream prefer-function-type rule is later improved to no longer flag this pattern natively, the test will fail and signal that the decorator suppression logic can be safely removed.

@francois-mora-sonarsource
Copy link
Contributor

[4D] getTopLevelCallExprs — potential helper candidate

getTopLevelCallExprs is a pure AST utility (no rule-specific logic) that walks top-level body statements to collect call expressions. Since it operates on the source code AST and has no domain coupling, it could be a candidate for helpers/vue.ts alongside isInsideVueSetupScript — especially if other Vue-related rules need similar access to top-level macro calls.

Minor observation; in-rule placement is acceptable if no broader reuse is anticipated.

@sonar-nigel
Copy link
Contributor Author

sonar-nigel bot commented Mar 6, 2026

Good observation. Since getTopLevelCallExprs is only used by this rule's decorator and no other Vue-related rules currently need access to top-level macro calls, I'll keep it in-rule for now. If a future rule requires similar access to top-level call expressions, it would be a natural time to promote it to helpers/vue.ts.

@sonar-nigel
Copy link
Contributor Author

sonar-nigel bot commented Mar 6, 2026

Good call. Added a sentinel test in unit.test.ts that runs tsEslintRules['prefer-function-type'] directly (before decoration) and asserts it raises an issue on a named-interface defineEmits type argument in a Vue <script setup> block. If upstream ever stops flagging this pattern natively, the sentinel will fail and signal that the suppression logic in decorator.ts can be safely removed.

@francois-mora-sonarsource
Copy link
Contributor

LGTM

@francois-mora-sonarsource francois-mora-sonarsource requested a review from a team March 6, 2026 13:29
@sonar-nigel
Copy link
Contributor Author

sonar-nigel bot commented Mar 6, 2026

Thank you for the review!

Vibe Bot and others added 5 commits March 10, 2026 14:38
Tests cover the scenario where an interface, inline type literal, or
named type alias with a single call signature is used as the type
argument of defineEmits<T>() inside a Vue 3 <script setup> block.
The tests verify that such patterns are not flagged (valid cases) and
that interfaces unrelated to defineEmits inside <script setup> are
still flagged (invalid case).

Relates to JS-1381
S6598 falsely flagged interfaces with only call signatures when used as
the type argument of defineEmits<T>() in Vue 3 <script setup> blocks.
This pattern is idiomatic and documented in Vue 3 — each call signature
represents a distinct emitted event and cannot be replaced by a function
type.

The fix extends decorator.ts with two sequential guards in the
interceptReport callback: Guard 1 (isInsideVueSetupScript) passes
through all reports outside Vue <script setup> unchanged. Guard 2
(isDefineEmitsTypeArg) suppresses reports only when the flagged node is
a defineEmits type argument. Three shapes are handled: inline type
literal, named type alias, and named interface. Regular S6598 violations
inside <script setup> unrelated to defineEmits are still reported.

The test for the remaining invalid case now includes the expected output
showing the fix applied (interface converted to function type alias).

Implementation follows the approved proposal guidelines. Relates to JS-1381.
Refactored decorator.ts to resolve 10 code quality issues reported by
SonarQube Next (S3776, S121, S134, S4204 x5, S4325).

Replaced estree-based type casts with proper TSESTree types from
@typescript-eslint/utils. Extracted getTopLevelCallExprs() helper using
Array.flatMap to eliminate 4-level nesting in isReferencedByDefineEmits
(fixes S3776 cognitive complexity and S134 nesting depth). All five
'any' type annotations replaced with TSESTree types and 'as unknown as'
casts (fixes S4204). The unnecessary 'as any' assertion on
greatGrandParent removed as TSESTree types make it redundant (fixes
S4325). The if (!name) guard eliminated since name is now typed as
string (non-optional) via TSESTree.TSInterfaceDeclaration.id.name (fixes
S121). TypeScript compilation verified error-free; tests unchanged.
Comment: **[4E] Missing upstream FP assertion**

The new Vue test cases use `rule` (already decorated). A useful addition would be a test that imports `tsEslintRules['prefer-function-type']` directly (before decoration) and asserts that the upstream rule *does* raise an issue on the `defineEmits` FP pattern — for example, an interface with only call signatures used as a `defineEmits<T>()` type argument in a Vue `<script setup>` block.

This acts as a sentinel: if the upstream `prefer-function-type` rule is later improved to no longer flag this pattern natively, the test will fail and signal that the decorator suppression logic can be safely removed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
The sentinel test for upstream prefer-function-type was missing the
required `output` field. ESLint's rule tester compares the actual
fixed output against the original input when `output` is omitted,
causing an assertion failure because the upstream rule applies an
autofix (converting interface to type alias).
@francois-mora-sonarsource francois-mora-sonarsource force-pushed the fix/JS-1381-fix-fp-on-s6598-interface-used-as-defineemits-type-argument-in-vue-script-setup-sonnet branch from 75188de to a4a4e5c Compare March 10, 2026 13:51
Copy link
Contributor

@vdiez vdiez left a comment

Choose a reason for hiding this comment

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

please clarify if the cast you are doing is safe considering all possible reported node types

context.report({
...reportDescriptor,
});
const node = (reportDescriptor as unknown as { node: TSESTree.TSCallSignatureDeclaration })
Copy link
Contributor

Choose a reason for hiding this comment

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

from what I see in prefer-function-type, the rule may report in TSESTree.TSThisType, TSCallSignatureDeclaration or TSConstructSignatureDeclaration. not sure this cast is 100% safe

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! The rule can indeed report on TSConstructSignatureDeclaration and TSThisType (with the unexpectedThisOnFunctionOnlyInterface message) in addition to TSCallSignatureDeclaration. Fixed by casting to TSESTree.Node first and adding a node.type !== 'TSCallSignatureDeclaration' guard that passes through all non-call-signature reports unchanged before narrowing the type.

…corator.ts:103

Comment: from what I see in `prefer-function-type`, the rule may report in `TSESTree.TSThisType`, `TSCallSignatureDeclaration` or `TSConstructSignatureDeclaration`. not sure this cast is 100% safe

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@sonar-nigel sonar-nigel bot requested a review from vdiez March 10, 2026 16:01
Removed unnecessary type assertion (S4325) in decorator.ts interceptReport
callback. After the TSCallSignatureDeclaration type guard, TypeScript narrows
the node type automatically, making the intermediate callSigNode variable and
its explicit cast redundant. Replaced with direct use of the narrowed node.
@sonarqube-next
Copy link

@sonar-nigel
Copy link
Contributor Author

sonar-nigel bot commented Mar 11, 2026

Good point. getTopLevelCallExprs is indeed a pure AST utility with no S6598-specific logic. For now, since no other Vue rules currently need access to top-level macro calls, I'll leave it in-rule per your note. If a second consumer emerges, moving it to helpers/vue.ts would be the natural next step.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants