Merge pull request #6437 from junichi11/php-gh-6432-named-args-after-…

…unpacking Fix wrong `IncorectNamedArgumentHintError` #6432
apache · Sep 15, 2023 · 841a4e8 · 841a4e8
2 parents f75da6e + 4a046ad
commit 841a4e8
Show file tree

Hide file tree

Showing 5 changed files with 74 additions and 95 deletions.
diff --git a/...or/src/org/netbeans/modules/php/editor/verification/IncorrectNamedArgumentsHintError.java b/...or/src/org/netbeans/modules/php/editor/verification/IncorrectNamedArgumentsHintError.java
@@ -58,7 +58,7 @@ public String getDisplayName() {
     @NbBundle.Messages({
         "# {0} - argument name",
         "IncorrectNamedArguments.desc.duplicate.name=Duplicate argument name: \"{0}\" already exists.",
-        "IncorrectNamedArguments.desc.combine.named.argument.and.argument.unpacking=Can't combine named arguments(name: arg) and argument unpacking(...).",
+        "IncorrectNamedArguments.desc.argument.unpacking.after.named.argument=Can't use argument unpacking(...) after named argument(name: arg).",
         "IncorrectNamedArguments.desc.positional.arguments.after.named.argument=Can't use positional argument after named argument.",
     })
     public void invoke(PHPRuleContext context, List<Hint> hints) {
@@ -80,12 +80,11 @@ public void invoke(PHPRuleContext context, List<Hint> hints) {
                 }
                 addHint(argument, Bundle.IncorrectNamedArguments_desc_duplicate_name(argument.getParameterName().getName()), hints);
             }
-            for (Map.Entry<NamedArgument, Variadic> entry : checkVisitor.getCombinedNamedArgumentsWithArgumentUnpacking().entrySet()) {
+            for (Variadic variadic : checkVisitor.getArgumentUnpackingAfterNamedArgument()) {
                 if (CancelSupport.getDefault().isCancelled()) {
                     return;
                 }
-                addHint(entry.getKey(), Bundle.IncorrectNamedArguments_desc_combine_named_argument_and_argument_unpacking(), hints);
-                addHint(entry.getValue(), Bundle.IncorrectNamedArguments_desc_combine_named_argument_and_argument_unpacking(), hints);
+                addHint(variadic, Bundle.IncorrectNamedArguments_desc_argument_unpacking_after_named_argument(), hints);
             }
             for (Expression argument : checkVisitor.getArgumentsAfterNamedArgument()) {
                 if (CancelSupport.getDefault().isCancelled()) {
@@ -115,7 +114,7 @@ private void addHint(ASTNode node, String description, List<Hint> hints, List<Hi
     private static final class CheckVisitor extends DefaultVisitor {
 
         private final Set<NamedArgument> duplicateNames = new HashSet<>();
-        private final Map<NamedArgument, Variadic> combinedNamedArgumentsWithArgumentUnpacking = new HashMap<>();
+        private final Set<Variadic> argumentUnpackingAfterNamedArgument = new HashSet<>();
         private final Set<Expression> argumentsAfterNamedArgument = new HashSet<>();
 
         @Override
@@ -152,7 +151,6 @@ private void processArguments(List<Expression> arguments) {
             }
             Set<String> names = new HashSet<>();
             NamedArgument firstNamedArgument = null;
-            Variadic variadic = null;
             for (Expression argument : arguments) {
                 if (CancelSupport.getDefault().isCancelled()) {
                     return;
@@ -173,8 +171,9 @@ private void processArguments(List<Expression> arguments) {
                         names.add(name);
                     }
                 } else if (argument instanceof Variadic) {
-                    if (variadic == null) {
-                        variadic = (Variadic) argument;
+                    if (firstNamedArgument != null) {
+                        // e.g. (a: 'arg', ...[])
+                        argumentUnpackingAfterNamedArgument.add((Variadic) argument);
                     }
                 } else {
                     if (firstNamedArgument != null) {
@@ -183,23 +182,18 @@ private void processArguments(List<Expression> arguments) {
                     }
                 }
             }
-            if (firstNamedArgument != null && variadic != null) {
-                // e.g. (a: 'arg', ...[]), (...[], a: 'arg')
-                combinedNamedArgumentsWithArgumentUnpacking.put(firstNamedArgument, variadic);
-            }
         }
 
         public Set<NamedArgument> getDuplicateNames() {
             return Collections.unmodifiableSet(duplicateNames);
         }
 
-        public Map<NamedArgument, Variadic> getCombinedNamedArgumentsWithArgumentUnpacking() {
-            return Collections.unmodifiableMap(combinedNamedArgumentsWithArgumentUnpacking);
-        }
-
         public Set<Expression> getArgumentsAfterNamedArgument() {
             return Collections.unmodifiableSet(argumentsAfterNamedArgument);
         }
 
+        public Set<Variadic> getArgumentUnpackingAfterNamedArgument() {
+            return Collections.unmodifiableSet(argumentUnpackingAfterNamedArgument);
+        }
     }
 }
diff --git a/...fication/IncorrectNamedArgumentsHintError/testNamedAndUnpack.php.testNamedAndUnpack.hints b/...fication/IncorrectNamedArgumentsHintError/testNamedAndUnpack.php.testNamedAndUnpack.hints
diff --git a/...ArgumentsHintError/testNamedAndUnpack.php → ...entsHintError/testUnpackingAfterNamed.php b/...ArgumentsHintError/testNamedAndUnpack.php → ...entsHintError/testUnpackingAfterNamed.php
@@ -22,66 +22,81 @@
 function test($a, $b, $c, $name) {
 }
 
-test(1, ...[1, 2, 3],);
-test(1, 2, 3, 4, ...[],);
-test(1, ...[2, 3, 4], ...[]);
-test(...[],);
-test(name: 'arg');
-test($test, name: 'arg');
+test(1, ...[1, 2, 3],); // OK
+test(1, 2, 3, 4, ...[],); // OK
+test(1, ...[2, 3, 4], ...[]); // OK
+test(...[],); // OK
+test(name: 'arg'); // OK
+test($test, name: 'arg'); // OK
 test(
-    ...[],
+    ...[], // OK
     name: 'arg'
 );
 test(
     1,
     2,
     3,
     name: 'arg',
-    ...[],
+    ...[], // NG
+);
+$uppacking = [1, 2, 3];
+test(...$unpacking, name: "arg"); // OK
+test(name: "arg", ...$unpacking); // NG
+test(
+        1,
+        b:2,
+        ...[2], // NG
+        ...[1] // NG
+);
+test(
+        a:1,
+        b:2,
+        ...[2], // NG
+        ...[1] // NG
 );
 
 class TestExample {
     public function test($a, $b, $c, $name): void {}
     public static function staticTest($a, $b, $c, $name): void {}
 
-    public function testNamedAndUnpack(): void {
+    public function testUnpackingAfterNamed(): void {
         $this->test(
-            ...[1, 2, 3],
+            ...[1, 2, 3], // OK
             name: 'arg'
         );
         $this->test(
             name: 'arg',
-            ...[1, 2, 3],
+            ...[1, 2, 3], // NG
         );
         self::staticTest(
-            ...[1, 2, 3],
+            ...[1, 2, 3], // OK
             name: 'arg'
         );
         self::staticTest(
             name: 'arg',
-            ...[1, 2, 3],
+            ...[1, 2, 3], // NG
         );
     }
 }
 
 $test = new Test(
     name: "test",
-    ...['a', 'b']
+    ...['a', 'b'] // NG
 );
 $test = new Test(
-    ...['a', 'b'],
+    ...['a', 'b'], // OK
     name: "test",
 );
 
 $anon = new class(
     name: "test",
-    ...['a', 'b'],
+    ...['a', 'b'], // NG
 ) {
     public function __construct($name, $a, $b) {}
 };
 
 $anon = new class(
-    ...['a', 'b'],
+    ...['a', 'b'], // OK
     name: "test",
 ) {
     public function __construct($name, $a, $b) {}

diff --git a/...ncorrectNamedArgumentsHintError/testUnpackingAfterNamed.php.testUnpackingAfterNamed.hints b/...ncorrectNamedArgumentsHintError/testUnpackingAfterNamed.php.testUnpackingAfterNamed.hints
@@ -0,0 +1,30 @@
+    ...[], // NG
+    -----
+HINT:Can't use argument unpacking(...) after named argument(name: arg).
+test(name: "arg", ...$unpacking); // NG
+                  -------------
+HINT:Can't use argument unpacking(...) after named argument(name: arg).
+        ...[2], // NG
+        ------
+HINT:Can't use argument unpacking(...) after named argument(name: arg).
+        ...[1] // NG
+        ------
+HINT:Can't use argument unpacking(...) after named argument(name: arg).
+        ...[2], // NG
+        ------
+HINT:Can't use argument unpacking(...) after named argument(name: arg).
+        ...[1] // NG
+        ------
+HINT:Can't use argument unpacking(...) after named argument(name: arg).
+            ...[1, 2, 3], // NG
+            ------------
+HINT:Can't use argument unpacking(...) after named argument(name: arg).
+            ...[1, 2, 3], // NG
+            ------------
+HINT:Can't use argument unpacking(...) after named argument(name: arg).
+    ...['a', 'b'] // NG
+    -------------
+HINT:Can't use argument unpacking(...) after named argument(name: arg).
+    ...['a', 'b'], // NG
+    -------------
+HINT:Can't use argument unpacking(...) after named argument(name: arg).
diff --git a/...rc/org/netbeans/modules/php/editor/verification/IncorrectNamedArgumentsHintErrorTest.java b/...rc/org/netbeans/modules/php/editor/verification/IncorrectNamedArgumentsHintErrorTest.java
@@ -46,8 +46,8 @@ public void testPositionalAfterNamedInAttributeArgList() throws Exception {
         checkHints(new IncorrectNamedArgumentsHintError(), "testPositionalAfterNamedInAttributeArgList.php");
     }
 
-    public void testNamedAndUnpack() throws Exception {
-        checkHints(new IncorrectNamedArgumentsHintError(), "testNamedAndUnpack.php");
+    public void testUnpackingAfterNamed() throws Exception {
+        checkHints(new IncorrectNamedArgumentsHintError(), "testUnpackingAfterNamed.php");
     }
 
 }