From 4a046ade489ed39c85ccb45f9e0a3c1e3c59efdb Mon Sep 17 00:00:00 2001 From: Junichi Yamamoto Date: Tue, 12 Sep 2023 13:36:20 +0900 Subject: [PATCH] Fix wrong `IncorectNamedArgumentHintError` #6432 - https://github.com/apache/netbeans/issues/6432 - Show errors when argument uppacking is used after named arguments ```php testUnpackingAfterNamed.php} | 49 +++++++++------ ...terNamed.php.testUnpackingAfterNamed.hints | 30 ++++++++++ .../IncorrectNamedArgumentsHintErrorTest.java | 4 +- 5 files changed, 74 insertions(+), 95 deletions(-) delete mode 100644 php/php.editor/test/unit/data/testfiles/verification/IncorrectNamedArgumentsHintError/testNamedAndUnpack.php.testNamedAndUnpack.hints rename php/php.editor/test/unit/data/testfiles/verification/IncorrectNamedArgumentsHintError/{testNamedAndUnpack.php => testUnpackingAfterNamed.php} (67%) create mode 100644 php/php.editor/test/unit/data/testfiles/verification/IncorrectNamedArgumentsHintError/testUnpackingAfterNamed.php.testUnpackingAfterNamed.hints diff --git a/php/php.editor/src/org/netbeans/modules/php/editor/verification/IncorrectNamedArgumentsHintError.java b/php/php.editor/src/org/netbeans/modules/php/editor/verification/IncorrectNamedArgumentsHintError.java index fd5239bb5baf..910240ab8aab 100644 --- a/php/php.editor/src/org/netbeans/modules/php/editor/verification/IncorrectNamedArgumentsHintError.java +++ b/php/php.editor/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 hints) { @@ -80,12 +80,11 @@ public void invoke(PHPRuleContext context, List hints) { } addHint(argument, Bundle.IncorrectNamedArguments_desc_duplicate_name(argument.getParameterName().getName()), hints); } - for (Map.Entry 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 hints, List duplicateNames = new HashSet<>(); - private final Map combinedNamedArgumentsWithArgumentUnpacking = new HashMap<>(); + private final Set argumentUnpackingAfterNamedArgument = new HashSet<>(); private final Set argumentsAfterNamedArgument = new HashSet<>(); @Override @@ -152,7 +151,6 @@ private void processArguments(List arguments) { } Set 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 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 arguments) { } } } - if (firstNamedArgument != null && variadic != null) { - // e.g. (a: 'arg', ...[]), (...[], a: 'arg') - combinedNamedArgumentsWithArgumentUnpacking.put(firstNamedArgument, variadic); - } } public Set getDuplicateNames() { return Collections.unmodifiableSet(duplicateNames); } - public Map getCombinedNamedArgumentsWithArgumentUnpacking() { - return Collections.unmodifiableMap(combinedNamedArgumentsWithArgumentUnpacking); - } - public Set getArgumentsAfterNamedArgument() { return Collections.unmodifiableSet(argumentsAfterNamedArgument); } + public Set getArgumentUnpackingAfterNamedArgument() { + return Collections.unmodifiableSet(argumentUnpackingAfterNamedArgument); + } } } diff --git a/php/php.editor/test/unit/data/testfiles/verification/IncorrectNamedArgumentsHintError/testNamedAndUnpack.php.testNamedAndUnpack.hints b/php/php.editor/test/unit/data/testfiles/verification/IncorrectNamedArgumentsHintError/testNamedAndUnpack.php.testNamedAndUnpack.hints deleted file mode 100644 index e3e605eb5219..000000000000 --- a/php/php.editor/test/unit/data/testfiles/verification/IncorrectNamedArgumentsHintError/testNamedAndUnpack.php.testNamedAndUnpack.hints +++ /dev/null @@ -1,60 +0,0 @@ - ...[], - ----- -HINT:Can't combine named arguments(name: arg) and argument unpacking(...). - name: 'arg' - ----------- -HINT:Can't combine named arguments(name: arg) and argument unpacking(...). - name: 'arg', - ----------- -HINT:Can't combine named arguments(name: arg) and argument unpacking(...). - ...[], - ----- -HINT:Can't combine named arguments(name: arg) and argument unpacking(...). - ...[1, 2, 3], - ------------ -HINT:Can't combine named arguments(name: arg) and argument unpacking(...). - name: 'arg' - ----------- -HINT:Can't combine named arguments(name: arg) and argument unpacking(...). - name: 'arg', - ----------- -HINT:Can't combine named arguments(name: arg) and argument unpacking(...). - ...[1, 2, 3], - ------------ -HINT:Can't combine named arguments(name: arg) and argument unpacking(...). - ...[1, 2, 3], - ------------ -HINT:Can't combine named arguments(name: arg) and argument unpacking(...). - name: 'arg' - ----------- -HINT:Can't combine named arguments(name: arg) and argument unpacking(...). - name: 'arg', - ----------- -HINT:Can't combine named arguments(name: arg) and argument unpacking(...). - ...[1, 2, 3], - ------------ -HINT:Can't combine named arguments(name: arg) and argument unpacking(...). - name: "test", - ------------ -HINT:Can't combine named arguments(name: arg) and argument unpacking(...). - ...['a', 'b'] - ------------- -HINT:Can't combine named arguments(name: arg) and argument unpacking(...). - ...['a', 'b'], - ------------- -HINT:Can't combine named arguments(name: arg) and argument unpacking(...). - name: "test", - ------------ -HINT:Can't combine named arguments(name: arg) and argument unpacking(...). - name: "test", - ------------ -HINT:Can't combine named arguments(name: arg) and argument unpacking(...). - ...['a', 'b'], - ------------- -HINT:Can't combine named arguments(name: arg) and argument unpacking(...). - ...['a', 'b'], - ------------- -HINT:Can't combine named arguments(name: arg) and argument unpacking(...). - name: "test", - ------------ -HINT:Can't combine named arguments(name: arg) and argument unpacking(...). diff --git a/php/php.editor/test/unit/data/testfiles/verification/IncorrectNamedArgumentsHintError/testNamedAndUnpack.php b/php/php.editor/test/unit/data/testfiles/verification/IncorrectNamedArgumentsHintError/testUnpackingAfterNamed.php similarity index 67% rename from php/php.editor/test/unit/data/testfiles/verification/IncorrectNamedArgumentsHintError/testNamedAndUnpack.php rename to php/php.editor/test/unit/data/testfiles/verification/IncorrectNamedArgumentsHintError/testUnpackingAfterNamed.php index 84d2ee2e4560..f19515d79b5a 100644 --- a/php/php.editor/test/unit/data/testfiles/verification/IncorrectNamedArgumentsHintError/testNamedAndUnpack.php +++ b/php/php.editor/test/unit/data/testfiles/verification/IncorrectNamedArgumentsHintError/testUnpackingAfterNamed.php @@ -22,14 +22,14 @@ 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( @@ -37,51 +37,66 @@ function test($a, $b, $c, $name) { 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/php/php.editor/test/unit/data/testfiles/verification/IncorrectNamedArgumentsHintError/testUnpackingAfterNamed.php.testUnpackingAfterNamed.hints b/php/php.editor/test/unit/data/testfiles/verification/IncorrectNamedArgumentsHintError/testUnpackingAfterNamed.php.testUnpackingAfterNamed.hints new file mode 100644 index 000000000000..35b791ec40fe --- /dev/null +++ b/php/php.editor/test/unit/data/testfiles/verification/IncorrectNamedArgumentsHintError/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/php/php.editor/test/unit/src/org/netbeans/modules/php/editor/verification/IncorrectNamedArgumentsHintErrorTest.java b/php/php.editor/test/unit/src/org/netbeans/modules/php/editor/verification/IncorrectNamedArgumentsHintErrorTest.java index b9cd72e71dbd..0458b87a9844 100644 --- a/php/php.editor/test/unit/src/org/netbeans/modules/php/editor/verification/IncorrectNamedArgumentsHintErrorTest.java +++ b/php/php.editor/test/unit/src/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"); } }