fix(promise): resolve review comments

Author: swwind

internal/plugins/promise/rules/no_nesting/no_nesting.go

@@ -30,14 +30,14 @@ func isPromiseCallback(node *ast.Node) bool {
 	return isThenOrCatchCall(parent)
 }
 
-// walkIdentifiers calls fn for each identifier name found anywhere in node's
+// walkIdentifiers calls fn for each identifier node found anywhere in node's
 // subtree, stopping early across the whole traversal if fn returns true.
-func walkIdentifiers(node *ast.Node, fn func(name string) bool) bool {
+func walkIdentifiers(node *ast.Node, fn func(identNode *ast.Node) bool) bool {
 	if node == nil {
 		return false
 	}
 	if node.Kind == ast.KindIdentifier {
-		return fn(node.AsIdentifier().Text)
+		return fn(node)
 	}
 	found := false
 	node.ForEachChild(func(child *ast.Node) bool {
@@ -61,18 +61,52 @@ func argsContainRef(callNode *ast.Node, fn *ast.Node) bool {
 		return false
 	}
 	body := fn.Body()
+	boundary := fn
+	if body != nil {
+		boundary = body
+	}
 	for _, arg := range args.Nodes {
 		found := false
-		walkIdentifiers(arg, func(name string) bool {
-			if utils.HasShadowingParameter(fn, name) {
-				found = true
-				return true
+		walkIdentifiers(arg, func(identNode *ast.Node) bool {
+			if utils.IsNonReferenceIdentifier(identNode) {
+				return false
+			}
+
+			name := identNode.AsIdentifier().Text
+
+			if name == "arguments" {
+				if utils.IsNameShadowedBetween(identNode, boundary, name) {
+					return false
+				}
+				isArgumentsShadowed := false
+				for curr := identNode.Parent; curr != nil && curr != fn; curr = curr.Parent {
+					if ast.IsFunctionLikeDeclaration(curr) && curr.Kind != ast.KindArrowFunction {
+						isArgumentsShadowed = true
+						break
+					}
+				}
+				if isArgumentsShadowed {
+					return false
+				}
+				if fn.Kind != ast.KindArrowFunction {
+					found = true
+					return true
+				}
+				return false
+			}
+
+			hasParam := utils.HasShadowingParameter(fn, name)
+			hasDecl := body != nil && (utils.HasShadowingDeclaration(body, name) || utils.HasHoistedVarDeclaration(body, name))
+			if !hasParam && !hasDecl {
+				return false
 			}
-			if body != nil && (utils.HasShadowingDeclaration(body, name) || utils.HasHoistedVarDeclaration(body, name)) {
-				found = true
-				return true
+
+			if utils.IsNameShadowedBetween(identNode, boundary, name) {
+				return false
 			}
-			return false
+
+			found = true
+			return true
 		})
 		if found {
 			return true

internal/plugins/promise/rules/no_nesting/no_nesting_extras_test.go

@@ -104,6 +104,9 @@ func TestNoNestingExtras(t *testing.T) {
 			{Code: `doThing().then(function(x) {
   return { get val() { return a.then(x) } };
 })`},
+
+			// ---- Dimension 4: implicit arguments binding in non-arrow callback ----
+			{Code: `doThing().then(function() { return foo.then(() => use(arguments)) })`},
 		},
 		[]rule_tester.InvalidTestCase{
 			// ---- Dimension 4: parenthesized callback — still reports inner .then ----
@@ -197,6 +200,30 @@ func TestNoNestingExtras(t *testing.T) {
 				Code:   `doThing().catch(function() { a.catch() })`,
 				Errors: []rule_tester.InvalidTestCaseError{{MessageId: "avoidNesting", Line: 1}},
 			},
+
+			// ---- Dimension 4: property access name is not a reference to outer binding ----
+			{
+				Code:   `doThing().then(user => getProfile().then(p => p.user))`,
+				Errors: []rule_tester.InvalidTestCaseError{{MessageId: "avoidNesting", Line: 1}},
+			},
+
+			// ---- Dimension 4: object key is not a reference to outer binding ----
+			{
+				Code:   `doThing().then(user => getProfile().then(p => ({ user: 1 })))`,
+				Errors: []rule_tester.InvalidTestCaseError{{MessageId: "avoidNesting", Line: 1}},
+			},
+
+			// ---- Dimension 4: inner parameter shadowing the outer binding ----
+			{
+				Code:   `doThing().then(a => inner.then(a => use(a)))`,
+				Errors: []rule_tester.InvalidTestCaseError{{MessageId: "avoidNesting", Line: 1}},
+			},
+
+			// ---- Dimension 4: implicit arguments binding shadowed by nested non-arrow callback ----
+			{
+				Code:   `doThing().then(function() { return foo.then(function() { use(arguments) }) })`,
+				Errors: []rule_tester.InvalidTestCaseError{{MessageId: "avoidNesting", Line: 1}},
+			},
 		},
 	)
 }

packages/rslint/src/configs/promise.ts

@@ -10,7 +10,7 @@ const recommended: RslintConfigEntry = {
     'promise/param-names': 'error',
     'promise/catch-or-return': 'error',
     // 'promise/no-native': 'off', // not implemented
-    'promise/no-nesting': 'warn',
+    // 'promise/no-nesting': 'warn', // not implemented
     // 'promise/no-promise-in-callback': 'warn', // not implemented
     // 'promise/no-callback-in-promise': 'warn', // not implemented
     'promise/avoid-new': 'off',