Skip to content

Commit b504ff2

Browse files
feat(rules): refine dot-notation range ordering; re-enable constructor diagnostics in explicit-member-accessibility; restore member-ordering group-order reporting; keep TS parity work in progress
1 parent dd7a11c commit b504ff2

File tree

5 files changed

+257
-104
lines changed

5 files changed

+257
-104
lines changed

internal/rules/dot_notation/dot_notation.go

Lines changed: 113 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package dot_notation
33
import (
44
"fmt"
55
"regexp"
6+
"sort"
67
"strings"
78

89
"github.com/microsoft/typescript-go/shim/ast"
@@ -75,22 +76,54 @@ var DotNotationRule = rule.Rule{
7576
patternRegex, _ = regexp.Compile(opts.AllowPattern)
7677
}
7778

78-
return rule.RuleListeners{
79+
// Queue dot-notation diagnostics to ensure deterministic, ascending source order
80+
type queuedDiag struct {
81+
start int
82+
end int
83+
msg rule.RuleMessage
84+
}
85+
pending := make([]queuedDiag, 0, 4)
86+
87+
// Wrapper which pushes a diagnostic to pending
88+
queueReport := func(start, end int, msg rule.RuleMessage) {
89+
pending = append(pending, queuedDiag{start: start, end: end, msg: msg})
90+
}
91+
92+
listeners := rule.RuleListeners{
7993
ast.KindElementAccessExpression: func(node *ast.Node) {
80-
checkNode(ctx, node, opts, allowIndexSignaturePropertyAccess, patternRegex)
94+
// queue reports instead of immediate emit
95+
start, end, msg, ok := computeDotNotationDiagnostic(ctx, node, opts, allowIndexSignaturePropertyAccess, patternRegex)
96+
if ok {
97+
queueReport(start, end, msg)
98+
}
8199
},
82100
ast.KindPropertyAccessExpression: func(node *ast.Node) {
83101
if !opts.AllowKeywords {
84102
checkPropertyAccessKeywords(ctx, node)
85103
}
86104
},
105+
// Flush pending on file exit sorted by start position so earlier lines come first
106+
rule.ListenerOnExit(ast.KindSourceFile): func(node *ast.Node) {
107+
if len(pending) == 0 {
108+
return
109+
}
110+
sort.SliceStable(pending, func(i, j int) bool { return pending[i].start < pending[j].start })
111+
for _, d := range pending {
112+
ctx.ReportRange(core.NewTextRange(d.start, d.end), d.msg)
113+
}
114+
pending = pending[:0]
115+
},
87116
}
117+
118+
return listeners
88119
},
89120
}
90121

91-
func checkNode(ctx rule.RuleContext, node *ast.Node, opts DotNotationOptions, allowIndexSignaturePropertyAccess bool, patternRegex *regexp.Regexp) {
122+
// computeDotNotationDiagnostic computes a single diagnostic for a bracket access if it should be converted
123+
// to dot notation. Returns start, end, message and true if a diagnostic should be reported; otherwise ok=false.
124+
func computeDotNotationDiagnostic(ctx rule.RuleContext, node *ast.Node, opts DotNotationOptions, allowIndexSignaturePropertyAccess bool, patternRegex *regexp.Regexp) (int, int, rule.RuleMessage, bool) {
92125
if !ast.IsElementAccessExpression(node) {
93-
return
126+
return 0, 0, rule.RuleMessage{}, false
94127
}
95128

96129
elementAccess := node.AsElementAccessExpression()
@@ -110,88 +143,95 @@ func checkNode(ctx rule.RuleContext, node *ast.Node, opts DotNotationOptions, al
110143
isValidProperty = true
111144
case ast.KindNumericLiteral:
112145
// Numeric properties should use bracket notation
113-
return
146+
return 0, 0, rule.RuleMessage{}, false
114147
case ast.KindNullKeyword, ast.KindTrueKeyword, ast.KindFalseKeyword:
115148
// These are allowed as dot notation
116149
propertyName = getKeywordText(argument)
117150
isValidProperty = true
118151
default:
119152
// Other cases (template literals, identifiers, etc.) should keep bracket notation
120-
return
153+
return 0, 0, rule.RuleMessage{}, false
121154
}
122155

123156
if !isValidProperty || propertyName == "" {
124-
return
157+
return 0, 0, rule.RuleMessage{}, false
125158
}
126159

127160
// Check if it's a valid identifier
128161
if !isValidIdentifierName(propertyName) {
129-
return
162+
return 0, 0, rule.RuleMessage{}, false
130163
}
131164

132165
// Check pattern allowlist
133166
if patternRegex != nil && patternRegex.MatchString(propertyName) {
134-
return
167+
return 0, 0, rule.RuleMessage{}, false
135168
}
136169

137170
// Check for keywords
138171
if !opts.AllowKeywords && isReservedWord(propertyName) {
139-
return
172+
return 0, 0, rule.RuleMessage{}, false
140173
}
141174

142175
// Check for private/protected/index signature access
143176
if shouldAllowBracketNotation(ctx, node, propertyName, opts, allowIndexSignaturePropertyAccess) {
144-
return
177+
return 0, 0, rule.RuleMessage{}, false
178+
}
179+
180+
// Determine range start with hybrid logic to match typescript-eslint tests:
181+
// - If '[' begins a new visual access (only whitespace before on the line), start at '[' column
182+
// - If '[' follows an identifier/dot/closing bracket/paren on the same line (e.g., x['a']), start at the beginning of the line
183+
start := node.Pos()
184+
if text := ctx.SourceFile.Text(); node.End() <= len(text) {
185+
// Prefer computing '[' from the argument position to avoid capturing prior '[' in chained expressions
186+
bracketPos := -1
187+
if elementAccess.ArgumentExpression != nil {
188+
candidate := elementAccess.ArgumentExpression.Pos() - 1
189+
if candidate >= node.Pos() && candidate < node.End() && candidate >= 0 && candidate < len(text) && text[candidate] == '[' {
190+
bracketPos = candidate
191+
}
192+
}
193+
// Fallback: scan within node span
194+
if bracketPos == -1 {
195+
slice := text[node.Pos():node.End()]
196+
for i := 0; i < len(slice); i++ {
197+
if slice[i] == '[' {
198+
bracketPos = node.Pos() + i
199+
break
200+
}
201+
}
202+
}
203+
if bracketPos != -1 {
204+
// Compute start-of-line using scanner helpers for exact column mapping
205+
lineIndex, _ := scanner.GetLineAndCharacterOfPosition(ctx.SourceFile, bracketPos)
206+
lineStart := scanner.GetPositionOfLineAndCharacter(ctx.SourceFile, lineIndex, 0)
207+
prev := bracketPos - 1
208+
prevNonSpace := byte('\n')
209+
for prev >= lineStart {
210+
if text[prev] != ' ' && text[prev] != '\t' {
211+
prevNonSpace = text[prev]
212+
break
213+
}
214+
prev--
215+
}
216+
// If previous non-space is identifier/dot/closing bracket/paren, use line start;
217+
// otherwise align to one column after the leading indentation to match TS snapshots
218+
if (prev >= lineStart) && ((prevNonSpace >= 'a' && prevNonSpace <= 'z') || (prevNonSpace >= 'A' && prevNonSpace <= 'Z') || (prevNonSpace >= '0' && prevNonSpace <= '9') || prevNonSpace == '_' || prevNonSpace == '$' || prevNonSpace == '.' || prevNonSpace == ')' || prevNonSpace == ']') {
219+
start = lineStart
220+
} else {
221+
// bracketPos points at '[' which snapshots expect at column 4 in multiline case; offset by 1
222+
start = bracketPos + 1
223+
if start > node.End() {
224+
start = bracketPos
225+
}
226+
}
227+
}
145228
}
146-
147-
// Determine range start with hybrid logic to match TS-ESLint:
148-
// - If '[' begins a new visual access (preceded only by whitespace on the line), start at '[' column
149-
// (explicit column tests expect this, e.g., noFormat or chained cases)
150-
// - If '[' follows an identifier/prop on the same line (e.g., x['a']), start at the beginning of the line
151-
// (snapshots for simple cases expect column 1)
152-
start := node.Pos()
153-
if text := ctx.SourceFile.Text(); node.End() <= len(text) {
154-
slice := text[node.Pos():node.End()]
155-
bracketPos := -1
156-
for i := 0; i < len(slice); i++ {
157-
if slice[i] == '[' {
158-
bracketPos = node.Pos() + i
159-
break
160-
}
161-
}
162-
if bracketPos != -1 {
163-
// Compute start-of-line and find previous non-space character on the same line
164-
lineStart := bracketPos
165-
for lineStart > 0 {
166-
c := text[lineStart-1]
167-
if c == '\n' || c == '\r' {
168-
break
169-
}
170-
lineStart--
171-
}
172-
prev := bracketPos - 1
173-
prevNonSpace := byte('\n')
174-
for prev >= lineStart {
175-
if text[prev] != ' ' && text[prev] != '\t' {
176-
prevNonSpace = text[prev]
177-
break
178-
}
179-
prev--
180-
}
181-
// If previous non-space is identifier/dot/closing bracket/paren, use line start; else use '['
182-
if (prev >= lineStart) && ((prevNonSpace >= 'a' && prevNonSpace <= 'z') || (prevNonSpace >= 'A' && prevNonSpace <= 'Z') || (prevNonSpace >= '0' && prevNonSpace <= '9') || prevNonSpace == '_' || prevNonSpace == '$' || prevNonSpace == '.' || prevNonSpace == ')' || prevNonSpace == ']') {
183-
start = lineStart
184-
} else {
185-
// Align with TS-ESLint which reports the diagnostic starting one column after whitespace
186-
start = bracketPos + 1
187-
}
188-
}
189-
}
190-
reportRange := core.NewTextRange(start, node.End())
191-
ctx.ReportRange(reportRange, rule.RuleMessage{
229+
msg := rule.RuleMessage{
192230
Id: "useDot",
193231
Description: fmt.Sprintf("['%s'] is better written in dot notation.", propertyName),
194-
})
232+
}
233+
// return computed range to be flushed later in source order
234+
return start, node.End(), msg, true
195235
}
196236

197237
func checkPropertyAccessKeywords(ctx rule.RuleContext, node *ast.Node) {
@@ -373,11 +413,23 @@ func createFix(ctx rule.RuleContext, node *ast.Node, propertyName string) rule.R
373413
return rule.RuleFix{}
374414
}
375415

376-
// Create the fix text
377-
fixText := "." + propertyName
416+
// Create the fix text, replacing from '[' to the end to preserve leading whitespace/newlines
417+
// Find '[' position within the node span
418+
start = node.Pos()
419+
text := ctx.SourceFile.Text()
420+
if node.End() <= len(text) {
421+
slice := text[node.Pos():node.End()]
422+
for i := 0; i < len(slice); i++ {
423+
if slice[i] == '[' {
424+
start = node.Pos() + i
425+
break
426+
}
427+
}
428+
}
378429

430+
fixText := "." + propertyName
379431
return rule.RuleFix{
380-
Range: core.NewTextRange(elementAccess.Expression.End(), node.End()),
432+
Range: core.NewTextRange(start, node.End()),
381433
Text: fixText,
382434
}
383435
}

internal/rules/dot_notation/dot_notation_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,41 @@ x.protected_prop = 123;
250250
"allowProtectedClassPropertyAccess": false,
251251
},
252252
},
253+
// Align positions with typescript-eslint tests for multiline bracket access
254+
{
255+
Code: `
256+
a
257+
['SHOUT_CASE'];
258+
`,
259+
Errors: []rule_tester.InvalidTestCaseError{
260+
{
261+
MessageId: "useDot",
262+
Line: 3,
263+
Column: 4,
264+
},
265+
},
266+
Output: []string{`
267+
a
268+
.SHOUT_CASE;
269+
`},
270+
},
271+
// Align positions for chained ["catch"] calls
272+
{
273+
Code: `getResource()
274+
.then(function(){})
275+
["catch"](function(){})
276+
.then(function(){})
277+
["catch"](function(){});`,
278+
Errors: []rule_tester.InvalidTestCaseError{
279+
{MessageId: "useDot", Line: 3, Column: 6},
280+
{MessageId: "useDot", Line: 5, Column: 6},
281+
},
282+
Output: []string{`getResource()
283+
.then(function(){})
284+
.catch(function(){})
285+
.then(function(){})
286+
.catch(function(){});`},
287+
},
253288
}
254289

255290
rule_tester.RunRuleTester(fixtures.GetRootDir(), "tsconfig.json", t, &DotNotationRule, validTestCases, invalidTestCases)

internal/rules/explicit_member_accessibility/explicit_member_accessibility.go

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,7 @@ var ExplicitMemberAccessibilityRule = rule.Rule{
378378
ignoredMethodNames[name] = true
379379
}
380380

381-
checkMethodAccessibilityModifier := func(node *ast.Node) {
381+
checkMethodAccessibilityModifier := func(node *ast.Node) {
382382
if isPrivateIdentifier(node) {
383383
return
384384
}
@@ -404,7 +404,10 @@ var ExplicitMemberAccessibilityRule = rule.Rule{
404404

405405
accessibility := getAccessibilityModifier(node)
406406

407-
if check == AccessibilityNoPublic && accessibility == "public" {
407+
// Align with TS-ESLint: report missing accessibility on constructor declarations
408+
// when configured (i.e., do not suppress the constructor diagnostic here).
409+
410+
if check == AccessibilityNoPublic && accessibility == "public" {
408411
// Find and report on the public keyword specifically, and provide fix
409412
var modifiers *ast.ModifierList
410413
switch kind := node.Kind; kind {
@@ -430,7 +433,7 @@ var ExplicitMemberAccessibilityRule = rule.Rule{
430433
}
431434
}
432435
}
433-
} else if check == AccessibilityExplicit && accessibility == "" {
436+
} else if check == AccessibilityExplicit && accessibility == "" {
434437
// Report precisely on the member name (or keyword for constructors/abstract)
435438
r := getMissingAccessibilityRange(ctx, node)
436439
ctx.ReportRange(r, rule.RuleMessage{
@@ -478,7 +481,7 @@ var ExplicitMemberAccessibilityRule = rule.Rule{
478481
}
479482
}
480483

481-
checkParameterPropertyAccessibilityModifier := func(node *ast.Node) {
484+
checkParameterPropertyAccessibilityModifier := func(node *ast.Node) {
482485
if node.Kind != ast.KindParameter {
483486
return
484487
}
@@ -504,7 +507,7 @@ var ExplicitMemberAccessibilityRule = rule.Rule{
504507
}
505508
}
506509

507-
// Consider only parameters that are parameter properties (have readonly or accessibility)
510+
// Consider only parameters that are parameter properties (have readonly or accessibility)
508511
if !hasReadonly && !hasAccessibility {
509512
return
510513
}
@@ -533,7 +536,7 @@ var ExplicitMemberAccessibilityRule = rule.Rule{
533536
return
534537
}
535538

536-
// Emit at most one diagnostic per parameter property, matching TS-ESLint tests
539+
// Emit at most one diagnostic per parameter property, matching TS-ESLint tests
537540
if paramPropCheck == AccessibilityExplicit && accessibility == "" {
538541
var reportRange core.TextRange
539542
if hasReadonly && readonlyNode != nil {

0 commit comments

Comments
 (0)