ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
* [ruby-core:79439] [Ruby trunk Bug#13196] Improve keyword argument errors when non-keyword arguments given
       [not found] <redmine.issue-13196.20170205201800@ruby-lang.org>
@ 2017-02-05 20:18 ` hi
  2017-02-06  3:24 ` [ruby-core:79448] " shevegen
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 11+ messages in thread
From: hi @ 2017-02-05 20:18 UTC (permalink / raw
  To: ruby-core

Issue #13196 has been reported by Olivier Lacan.

----------------------------------------
Bug #13196: Improve keyword argument errors when non-keyword arguments given
https://bugs.ruby-lang.org/issues/13196

* Author: Olivier Lacan
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: 
* Backport: 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: UNKNOWN
----------------------------------------
Given the following method definition:


```ruby
def explode(code:)
  puts "Boom!"
end
```

If a Ruby user doesn't provide any arguments when calling the `explode` method, the following helpful feedback is given:

```ruby
explode
ArgumentError: missing keyword: code
```

But when a Ruby user mistakenly provides a regular argument, the exception message is obtuse and unhelpful: 

```ruby
explode "1234"
ArgumentError: wrong number of arguments (given 1, expected 0)
```

This does not provide information to properly recover from the error. Worse, it's incorrect. It is not true that the method expected 0 arguments. The method expected 1 keyword argument.

Instead, Ruby should respond something like: 

```ruby
explode "1234"
ArgumentError: missing keyword: code, given "1234" which is not a keyword argument.
```

One could argue that this situation would call for a different error class, perhaps a `KeywordArgumentError` that would inherit from `ArgumentError`, but that would extend the scope of this feature request a bit too far in my mind. 



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [ruby-core:79448] [Ruby trunk Bug#13196] Improve keyword argument errors when non-keyword arguments given
       [not found] <redmine.issue-13196.20170205201800@ruby-lang.org>
  2017-02-05 20:18 ` [ruby-core:79439] [Ruby trunk Bug#13196] Improve keyword argument errors when non-keyword arguments given hi
@ 2017-02-06  3:24 ` shevegen
  2017-02-06  4:09 ` [ruby-core:79451] " nobu
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 11+ messages in thread
From: shevegen @ 2017-02-06  3:24 UTC (permalink / raw
  To: ruby-core

Issue #13196 has been updated by Robert A. Heiler.


I guess the error-reporting there did not yet account for the possibility that keyword arguments can be
mandatory too, so I agree that the message "wrong number of arguments (given 1, expected 0)" appears to
be incorrect, since one indeed has to pass a mandatory argument to that method. I assume that when
keyword args were added, this behaviour was probably not yet been noticed. I have not seen code like
"foo:" in any method definition yet either, though.

----------------------------------------
Bug #13196: Improve keyword argument errors when non-keyword arguments given
https://bugs.ruby-lang.org/issues/13196#change-62880

* Author: Olivier Lacan
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: 
* Backport: 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: UNKNOWN
----------------------------------------
Given the following method definition:


```ruby
def explode(code:)
  puts "Boom!"
end
```

If a Ruby user doesn't provide any arguments when calling the `explode` method, the following helpful feedback is given:

```ruby
explode
ArgumentError: missing keyword: code
```

But when a Ruby user mistakenly provides a regular argument, the exception message is obtuse and unhelpful: 

```ruby
explode "1234"
ArgumentError: wrong number of arguments (given 1, expected 0)
```

This does not provide information to properly recover from the error. Worse, it's incorrect. It is not true that the method expected 0 arguments. The method expected 1 keyword argument.

Instead, Ruby should respond something like: 

```ruby
explode "1234"
ArgumentError: missing keyword: code, given "1234" which is not a keyword argument.
```

One could argue that this situation would call for a different error class, perhaps a `KeywordArgumentError` that would inherit from `ArgumentError`, but that would extend the scope of this feature request a bit too far in my mind. 



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [ruby-core:79451] [Ruby trunk Bug#13196] Improve keyword argument errors when non-keyword arguments given
       [not found] <redmine.issue-13196.20170205201800@ruby-lang.org>
  2017-02-05 20:18 ` [ruby-core:79439] [Ruby trunk Bug#13196] Improve keyword argument errors when non-keyword arguments given hi
  2017-02-06  3:24 ` [ruby-core:79448] " shevegen
@ 2017-02-06  4:09 ` nobu
  2017-02-09 20:00 ` [ruby-core:79488] " sto.mar
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 11+ messages in thread
From: nobu @ 2017-02-06  4:09 UTC (permalink / raw
  To: ruby-core

Issue #13196 has been updated by Nobuyoshi Nakada.


How about this?

```
$ ./miniruby -e 'def explode(code:)end' -e 'explode(1)'
-e:1:in `explode': wrong number of arguments (given 1, expected 0 with required keyword code) (ArgumentError)
	from -e:2:in `<main>'
```

```diff
diff --git a/vm_args.c b/vm_args.c
index 6cded80924..76516f60e0 100644
--- a/vm_args.c
+++ b/vm_args.c
@@ -725,7 +725,25 @@ raise_argument_error(rb_thread_t *th, const rb_iseq_t *iseq, const VALUE exc)
 static void
 argument_arity_error(rb_thread_t *th, const rb_iseq_t *iseq, const int miss_argc, const int min_argc, const int max_argc)
 {
-    raise_argument_error(th, iseq, rb_arity_error_new(miss_argc, min_argc, max_argc));
+    VALUE exc = rb_arity_error_new(miss_argc, min_argc, max_argc);
+    if (iseq->body->param.flags.has_kw) {
+	const struct rb_iseq_param_keyword *const kw = iseq->body->param.keyword;
+	const ID *keywords = kw->table;
+	int req_key_num = kw->required_num;
+	if (req_key_num > 0) {
+	    VALUE mesg = rb_attr_get(exc, idMesg);
+	    rb_str_resize(mesg, RSTRING_LEN(mesg)-1);
+	    rb_str_cat_cstr(mesg, " with required keyword");
+	    if (req_key_num > 1) rb_str_cat_cstr(mesg, "s");
+	    do {
+		rb_str_cat_cstr(mesg, " ");
+		rb_str_append(mesg, rb_id2str(*keywords++));
+		rb_str_cat_cstr(mesg, ",");
+	    } while (--req_key_num);
+	    RSTRING_PTR(mesg)[RSTRING_LEN(mesg)-1] = ')';
+	}
+    }
+    raise_argument_error(th, iseq, exc);
 }
 
 static void
```

----------------------------------------
Bug #13196: Improve keyword argument errors when non-keyword arguments given
https://bugs.ruby-lang.org/issues/13196#change-62883

* Author: Olivier Lacan
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: 
* Backport: 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: UNKNOWN
----------------------------------------
Given the following method definition:


```ruby
def explode(code:)
  puts "Boom!"
end
```

If a Ruby user doesn't provide any arguments when calling the `explode` method, the following helpful feedback is given:

```ruby
explode
ArgumentError: missing keyword: code
```

But when a Ruby user mistakenly provides a regular argument, the exception message is obtuse and unhelpful: 

```ruby
explode "1234"
ArgumentError: wrong number of arguments (given 1, expected 0)
```

This does not provide information to properly recover from the error. Worse, it's incorrect. It is not true that the method expected 0 arguments. The method expected 1 keyword argument.

Instead, Ruby should respond something like: 

```ruby
explode "1234"
ArgumentError: missing keyword: code, given "1234" which is not a keyword argument.
```

One could argue that this situation would call for a different error class, perhaps a `KeywordArgumentError` that would inherit from `ArgumentError`, but that would extend the scope of this feature request a bit too far in my mind. 



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [ruby-core:79488] [Ruby trunk Bug#13196] Improve keyword argument errors when non-keyword arguments given
       [not found] <redmine.issue-13196.20170205201800@ruby-lang.org>
                   ` (2 preceding siblings ...)
  2017-02-06  4:09 ` [ruby-core:79451] " nobu
@ 2017-02-09 20:00 ` sto.mar
  2017-02-26  3:26 ` [ruby-core:79778] " hi
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 11+ messages in thread
From: sto.mar @ 2017-02-09 20:00 UTC (permalink / raw
  To: ruby-core

Issue #13196 has been updated by Marcus Stollsteimer.


Nobuyoshi Nakada wrote:
> wrong number of arguments (given 1, expected 0 with required keyword code)

IMO still unclear, sounds somewhat like _"given 1 argument with required keyword code, but expected 0"_. Also, `code` would have to be quoted  (_keyword `code'_) or otherwise marked as an identifier, or it could easily be interpreted as normal part of the message.

Maybe something like this(?):

```
wrong number of arguments (given 1, expected 0; missing keywords: code, foo)
```

which would only require a slight change in your patch. I'm not sure whether it would be good enough, though.

----------------------------------------
Bug #13196: Improve keyword argument errors when non-keyword arguments given
https://bugs.ruby-lang.org/issues/13196#change-62936

* Author: Olivier Lacan
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: 
* Backport: 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: UNKNOWN
----------------------------------------
Given the following method definition:


```ruby
def explode(code:)
  puts "Boom!"
end
```

If a Ruby user doesn't provide any arguments when calling the `explode` method, the following helpful feedback is given:

```ruby
explode
ArgumentError: missing keyword: code
```

But when a Ruby user mistakenly provides a regular argument, the exception message is obtuse and unhelpful: 

```ruby
explode "1234"
ArgumentError: wrong number of arguments (given 1, expected 0)
```

This does not provide information to properly recover from the error. Worse, it's incorrect. It is not true that the method expected 0 arguments. The method expected 1 keyword argument.

Instead, Ruby should respond something like: 

```ruby
explode "1234"
ArgumentError: missing keyword: code, given "1234" which is not a keyword argument.
```

One could argue that this situation would call for a different error class, perhaps a `KeywordArgumentError` that would inherit from `ArgumentError`, but that would extend the scope of this feature request a bit too far in my mind. 



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [ruby-core:79778] [Ruby trunk Bug#13196] Improve keyword argument errors when non-keyword arguments given
       [not found] <redmine.issue-13196.20170205201800@ruby-lang.org>
                   ` (3 preceding siblings ...)
  2017-02-09 20:00 ` [ruby-core:79488] " sto.mar
@ 2017-02-26  3:26 ` hi
  2017-02-26  3:40 ` [ruby-core:79779] " hi
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 11+ messages in thread
From: hi @ 2017-02-26  3:26 UTC (permalink / raw
  To: ruby-core

Issue #13196 has been updated by Olivier Lacan.

File missing_kwargs.diff added

Marcus Stollsteimer wrote:
> wrong number of arguments (given 1, expected 0; missing keywords: code, foo)

I agree that this is clearer so I slightly modified Nobu's patch:

```diff
diff --git a/vm_args.c b/vm_args.c
index 6cded80924..76516f60e0 100644
--- a/vm_args.c
+++ b/vm_args.c
@@ -725,7 +725,25 @@ raise_argument_error(rb_thread_t *th, const rb_iseq_t *iseq, const VALUE exc)
 static void
 argument_arity_error(rb_thread_t *th, const rb_iseq_t *iseq, const int miss_argc, const int min_argc, const int max_argc)
 {
-    raise_argument_error(th, iseq, rb_arity_error_new(miss_argc, min_argc, max_argc));
+    VALUE exc = rb_arity_error_new(miss_argc, min_argc, max_argc);
+    if (iseq->body->param.flags.has_kw) {
+   const struct rb_iseq_param_keyword *const kw = iseq->body->param.keyword;
+   const ID *keywords = kw->table;
+   int req_key_num = kw->required_num;
+   if (req_key_num > 0) {
+       VALUE mesg = rb_attr_get(exc, idMesg);
+       rb_str_resize(mesg, RSTRING_LEN(mesg)-1);
+       rb_str_cat_cstr(mesg, "; required keyword");
+       if (req_key_num > 1) rb_str_cat_cstr(mesg, "s");
+       do {
+       rb_str_cat_cstr(mesg, ": ");
+       rb_str_append(mesg, rb_id2str(*keywords++));
+       rb_str_cat_cstr(mesg, ",");
+       } while (--req_key_num);
+       RSTRING_PTR(mesg)[RSTRING_LEN(mesg)-1] = ')';
+   }
+    }
+    raise_argument_error(th, iseq, exc);
 }

 static void
```


----------------------------------------
Bug #13196: Improve keyword argument errors when non-keyword arguments given
https://bugs.ruby-lang.org/issues/13196#change-63204

* Author: Olivier Lacan
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: 
* Backport: 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: UNKNOWN
----------------------------------------
Given the following method definition:


```ruby
def explode(code:)
  puts "Boom!"
end
```

If a Ruby user doesn't provide any arguments when calling the `explode` method, the following helpful feedback is given:

```ruby
explode
ArgumentError: missing keyword: code
```

But when a Ruby user mistakenly provides a regular argument, the exception message is obtuse and unhelpful: 

```ruby
explode "1234"
ArgumentError: wrong number of arguments (given 1, expected 0)
```

This does not provide information to properly recover from the error. Worse, it's incorrect. It is not true that the method expected 0 arguments. The method expected 1 keyword argument.

Instead, Ruby should respond something like: 

```ruby
explode "1234"
ArgumentError: missing keyword: code, given "1234" which is not a keyword argument.
```

One could argue that this situation would call for a different error class, perhaps a `KeywordArgumentError` that would inherit from `ArgumentError`, but that would extend the scope of this feature request a bit too far in my mind. 

---Files--------------------------------
missing_kwargs.diff (1.2 KB)


-- 
https://bugs.ruby-lang.org/

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [ruby-core:79779] [Ruby trunk Bug#13196] Improve keyword argument errors when non-keyword arguments given
       [not found] <redmine.issue-13196.20170205201800@ruby-lang.org>
                   ` (4 preceding siblings ...)
  2017-02-26  3:26 ` [ruby-core:79778] " hi
@ 2017-02-26  3:40 ` hi
  2017-02-26 21:07 ` [ruby-core:79791] " sto.mar
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 11+ messages in thread
From: hi @ 2017-02-26  3:40 UTC (permalink / raw
  To: ruby-core

Issue #13196 has been updated by Olivier Lacan.


Although I find Nobu's patch excellent, it still bothers me that the exception says `expected 0`. 

Keyword arguments do increase a method's arity just like regular arguments, so the message is both confusing *and* disingenuous.

Given the following definition:

```ruby
def explode(code:)
  puts "Boom!"
end
```

And the following call:

```ruby
explode "1234"
```

I would much rather receive the following exception message:

```
ArgumentError: invalid argument "1234" (expected keyword arguments: :code, :foo)
```

First observation, since the keywords are symbols, they should be referred to as symbols — otherwise users may incorrectly try to pass variables named `code` and `foo` to recover from the error. Yes, I know it's silly but that appears to be what the exception message suggests.

Second observation, while it could be unwieldy to display the submitted arguments (especially if there are many and their representation is long), it also makes for a much clearer context for the feedback being given. 

An alternative would be:

```
ArgumentError: 1 unexpected non-keyword argument (expected keyword arguments: :code, :foo)
```

----------------------------------------
Bug #13196: Improve keyword argument errors when non-keyword arguments given
https://bugs.ruby-lang.org/issues/13196#change-63205

* Author: Olivier Lacan
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: 
* Backport: 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: UNKNOWN
----------------------------------------
Given the following method definition:


```ruby
def explode(code:)
  puts "Boom!"
end
```

If a Ruby user doesn't provide any arguments when calling the `explode` method, the following helpful feedback is given:

```ruby
explode
ArgumentError: missing keyword: code
```

But when a Ruby user mistakenly provides a regular argument, the exception message is obtuse and unhelpful: 

```ruby
explode "1234"
ArgumentError: wrong number of arguments (given 1, expected 0)
```

This does not provide information to properly recover from the error. Worse, it's incorrect. It is not true that the method expected 0 arguments. The method expected 1 keyword argument.

Instead, Ruby should respond something like: 

```ruby
explode "1234"
ArgumentError: missing keyword: code, given "1234" which is not a keyword argument.
```

One could argue that this situation would call for a different error class, perhaps a `KeywordArgumentError` that would inherit from `ArgumentError`, but that would extend the scope of this feature request a bit too far in my mind. 

---Files--------------------------------
missing_kwargs.diff (1.2 KB)


-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [ruby-core:79791] [Ruby trunk Bug#13196] Improve keyword argument errors when non-keyword arguments given
       [not found] <redmine.issue-13196.20170205201800@ruby-lang.org>
                   ` (5 preceding siblings ...)
  2017-02-26  3:40 ` [ruby-core:79779] " hi
@ 2017-02-26 21:07 ` sto.mar
  2017-02-27  1:50 ` [ruby-core:79794] " duerst
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 11+ messages in thread
From: sto.mar @ 2017-02-26 21:07 UTC (permalink / raw
  To: ruby-core

Issue #13196 has been updated by Marcus Stollsteimer.


Olivier Lacan wrote:
> ```diff
> +   if (req_key_num > 0) {
> +       VALUE mesg = rb_attr_get(exc, idMesg);
> +       rb_str_resize(mesg, RSTRING_LEN(mesg)-1);
> +       rb_str_cat_cstr(mesg, "; required keyword");
> +       if (req_key_num > 1) rb_str_cat_cstr(mesg, "s");
> +       do {
> +       rb_str_cat_cstr(mesg, ": ");
> +       rb_str_append(mesg, rb_id2str(*keywords++));
> +       rb_str_cat_cstr(mesg, ",");
> +       } while (--req_key_num);
> ```

* I think `rb_str_cat_cstr(mesg, ": ");` in the while-loop doesn't work; ":" must be added before the loop, in the loop only " " to separate different entries

* suggestion: s/keyword/keyword argument/, since "keyword" might be confused with language keywords like `def`, `end`, ...

Regarding `code` vs. `:code`, IMHO for beginners it's much easier to understand without leading ":", similar to the usage in the call sequence, it's _not_ `explode(:code: 123)` but `explode(code: 123)`, and in the method body.

----------------------------------------
Bug #13196: Improve keyword argument errors when non-keyword arguments given
https://bugs.ruby-lang.org/issues/13196#change-63217

* Author: Olivier Lacan
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: 
* Backport: 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: UNKNOWN
----------------------------------------
Given the following method definition:


```ruby
def explode(code:)
  puts "Boom!"
end
```

If a Ruby user doesn't provide any arguments when calling the `explode` method, the following helpful feedback is given:

```ruby
explode
ArgumentError: missing keyword: code
```

But when a Ruby user mistakenly provides a regular argument, the exception message is obtuse and unhelpful: 

```ruby
explode "1234"
ArgumentError: wrong number of arguments (given 1, expected 0)
```

This does not provide information to properly recover from the error. Worse, it's incorrect. It is not true that the method expected 0 arguments. The method expected 1 keyword argument.

Instead, Ruby should respond something like: 

```ruby
explode "1234"
ArgumentError: missing keyword: code, given "1234" which is not a keyword argument.
```

One could argue that this situation would call for a different error class, perhaps a `KeywordArgumentError` that would inherit from `ArgumentError`, but that would extend the scope of this feature request a bit too far in my mind. 

---Files--------------------------------
missing_kwargs.diff (1.2 KB)


-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [ruby-core:79794] [Ruby trunk Bug#13196] Improve keyword argument errors when non-keyword arguments given
       [not found] <redmine.issue-13196.20170205201800@ruby-lang.org>
                   ` (6 preceding siblings ...)
  2017-02-26 21:07 ` [ruby-core:79791] " sto.mar
@ 2017-02-27  1:50 ` duerst
  2017-03-16  8:10 ` [ruby-core:80184] " hi
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 11+ messages in thread
From: duerst @ 2017-02-27  1:50 UTC (permalink / raw
  To: ruby-core

Issue #13196 has been updated by Martin Dürst.


Marcus Stollsteimer wrote:

> Regarding `code` vs. `:code`, IMHO for beginners it's much easier to understand without leading ":", similar to the usage in the call sequence, it's _not_ `explode(:code: 123)` but `explode(code: 123)`, and in the method body.

Yes, actually, if a colon is needed at all, I'd put it at the end of the keyword(s), because that's how it appears in the method invocation:

```
ArgumentError: 1 unexpected non-keyword argument (expected keyword arguments: code:, foo:)
```

But I think the colons are unnecessary; the name of the arguments is `code` and `foo`; that these names are expressed as symbols in *some* contexts and that symbols are denoted with colons before or after are syntactic details depending on the context.

----------------------------------------
Bug #13196: Improve keyword argument errors when non-keyword arguments given
https://bugs.ruby-lang.org/issues/13196#change-63222

* Author: Olivier Lacan
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: 
* Backport: 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: UNKNOWN
----------------------------------------
Given the following method definition:


```ruby
def explode(code:)
  puts "Boom!"
end
```

If a Ruby user doesn't provide any arguments when calling the `explode` method, the following helpful feedback is given:

```ruby
explode
ArgumentError: missing keyword: code
```

But when a Ruby user mistakenly provides a regular argument, the exception message is obtuse and unhelpful: 

```ruby
explode "1234"
ArgumentError: wrong number of arguments (given 1, expected 0)
```

This does not provide information to properly recover from the error. Worse, it's incorrect. It is not true that the method expected 0 arguments. The method expected 1 keyword argument.

Instead, Ruby should respond something like: 

```ruby
explode "1234"
ArgumentError: missing keyword: code, given "1234" which is not a keyword argument.
```

One could argue that this situation would call for a different error class, perhaps a `KeywordArgumentError` that would inherit from `ArgumentError`, but that would extend the scope of this feature request a bit too far in my mind. 

---Files--------------------------------
missing_kwargs.diff (1.2 KB)


-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [ruby-core:80184] [Ruby trunk Bug#13196] Improve keyword argument errors when non-keyword arguments given
       [not found] <redmine.issue-13196.20170205201800@ruby-lang.org>
                   ` (7 preceding siblings ...)
  2017-02-27  1:50 ` [ruby-core:79794] " duerst
@ 2017-03-16  8:10 ` hi
  2017-05-19  6:07 ` [ruby-core:81245] " matz
  2017-07-04  0:31 ` [ruby-core:81890] " hi
  10 siblings, 0 replies; 11+ messages in thread
From: hi @ 2017-03-16  8:10 UTC (permalink / raw
  To: ruby-core

Issue #13196 has been updated by olivierlacan (Olivier Lacan).

File missing_kwargs.diff added

stomar (Marcus Stollsteimer) wrote:
> * I think `rb_str_cat_cstr(mesg, ": ");` in the while-loop doesn't work; ":" must be added before the loop, in the loop only " " to separate different entries

Woops, good catch. Fixed in a new attached patch.

> * suggestion: s/keyword/keyword argument/, since "keyword" might be confused with language keywords like `def`, `end`, ...

Can't do that since the normal kwargs error is:
> ArgumentError: missing keywords: code, token

> Regarding `code` vs. `:code`, IMHO for beginners it's much easier to understand without leading ":", similar to the usage in the call sequence, it's _not_ `explode(:code: 123)` but `explode(code: 123)`, and in the method body.
I've changed my mind on this and I agree. Especially since the above existing error lists references the keyword arguments without colons.

duerst (Martin Dürst) wrote: 
> Yes, actually, if a colon is needed at all, I'd put it at the end of the keyword(s), because that's how it appears in the method invocation:

Looks a bit odd, doesn't it? I could be convinced but this is beyond the scope of this patch and issue since there's existing error messages using no colons at all. 

Here's the patch tested on trunk (2.5.0 dev): 

```
irb(main):001:0> def explode(code:,token:)
irb(main):002:1> puts "Boom!"
irb(main):003:1> end
=> :explode
irb(main):004:0> explode
ArgumentError: missing keywords: code, token
	from (irb):1:in `explode'
	from (irb):4
	from /usr/local/bin/irb:11:in `<main>'
irb(main):005:0> explode "1234"
ArgumentError: wrong number of arguments (given 1, expected 0; required keywords: code, token)
	from (irb):1:in `explode'
	from (irb):5
	from /usr/local/bin/irb:11:in `<main>'
irb(main):006:0> RUBY_VERSION
=> "2.5.0"
```


----------------------------------------
Bug #13196: Improve keyword argument errors when non-keyword arguments given
https://bugs.ruby-lang.org/issues/13196#change-63626

* Author: olivierlacan (Olivier Lacan)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: 
* Backport: 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: UNKNOWN
----------------------------------------
Given the following method definition:


```ruby
def explode(code:)
  puts "Boom!"
end
```

If a Ruby user doesn't provide any arguments when calling the `explode` method, the following helpful feedback is given:

```ruby
explode
ArgumentError: missing keyword: code
```

But when a Ruby user mistakenly provides a regular argument, the exception message is obtuse and unhelpful: 

```ruby
explode "1234"
ArgumentError: wrong number of arguments (given 1, expected 0)
```

This does not provide information to properly recover from the error. Worse, it's incorrect. It is not true that the method expected 0 arguments. The method expected 1 keyword argument.

Instead, Ruby should respond something like: 

```ruby
explode "1234"
ArgumentError: missing keyword: code, given "1234" which is not a keyword argument.
```

One could argue that this situation would call for a different error class, perhaps a `KeywordArgumentError` that would inherit from `ArgumentError`, but that would extend the scope of this feature request a bit too far in my mind. 

---Files--------------------------------
missing_kwargs.diff (1.2 KB)
missing_kwargs.diff (1.23 KB)


-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [ruby-core:81245] [Ruby trunk Bug#13196] Improve keyword argument errors when non-keyword arguments given
       [not found] <redmine.issue-13196.20170205201800@ruby-lang.org>
                   ` (8 preceding siblings ...)
  2017-03-16  8:10 ` [ruby-core:80184] " hi
@ 2017-05-19  6:07 ` matz
  2017-07-04  0:31 ` [ruby-core:81890] " hi
  10 siblings, 0 replies; 11+ messages in thread
From: matz @ 2017-05-19  6:07 UTC (permalink / raw
  To: ruby-core

Issue #13196 has been updated by matz (Yukihiro Matsumoto).


Agreed with better message description.

Matz.


----------------------------------------
Bug #13196: Improve keyword argument errors when non-keyword arguments given
https://bugs.ruby-lang.org/issues/13196#change-64902

* Author: olivierlacan (Olivier Lacan)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: 
* Backport: 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: UNKNOWN
----------------------------------------
Given the following method definition:


```ruby
def explode(code:)
  puts "Boom!"
end
```

If a Ruby user doesn't provide any arguments when calling the `explode` method, the following helpful feedback is given:

```ruby
explode
ArgumentError: missing keyword: code
```

But when a Ruby user mistakenly provides a regular argument, the exception message is obtuse and unhelpful: 

```ruby
explode "1234"
ArgumentError: wrong number of arguments (given 1, expected 0)
```

This does not provide information to properly recover from the error. Worse, it's incorrect. It is not true that the method expected 0 arguments. The method expected 1 keyword argument.

Instead, Ruby should respond something like: 

```ruby
explode "1234"
ArgumentError: missing keyword: code, given "1234" which is not a keyword argument.
```

One could argue that this situation would call for a different error class, perhaps a `KeywordArgumentError` that would inherit from `ArgumentError`, but that would extend the scope of this feature request a bit too far in my mind. 

---Files--------------------------------
missing_kwargs.diff (1.2 KB)
missing_kwargs.diff (1.23 KB)


-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [ruby-core:81890] [Ruby trunk Bug#13196] Improve keyword argument errors when non-keyword arguments given
       [not found] <redmine.issue-13196.20170205201800@ruby-lang.org>
                   ` (9 preceding siblings ...)
  2017-05-19  6:07 ` [ruby-core:81245] " matz
@ 2017-07-04  0:31 ` hi
  10 siblings, 0 replies; 11+ messages in thread
From: hi @ 2017-07-04  0:31 UTC (permalink / raw
  To: ruby-core

Issue #13196 has been updated by olivierlacan (Olivier Lacan).


matz (Yukihiro Matsumoto) wrote:
> Agreed with better message description.

Thank you. Is there anything I can do to help move this issue along? Is the patch sufficient? 

----------------------------------------
Bug #13196: Improve keyword argument errors when non-keyword arguments given
https://bugs.ruby-lang.org/issues/13196#change-65618

* Author: olivierlacan (Olivier Lacan)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: 
* Backport: 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: UNKNOWN
----------------------------------------
Given the following method definition:


```ruby
def explode(code:)
  puts "Boom!"
end
```

If a Ruby user doesn't provide any arguments when calling the `explode` method, the following helpful feedback is given:

```ruby
explode
ArgumentError: missing keyword: code
```

But when a Ruby user mistakenly provides a regular argument, the exception message is obtuse and unhelpful: 

```ruby
explode "1234"
ArgumentError: wrong number of arguments (given 1, expected 0)
```

This does not provide information to properly recover from the error. Worse, it's incorrect. It is not true that the method expected 0 arguments. The method expected 1 keyword argument.

Instead, Ruby should respond something like: 

```ruby
explode "1234"
ArgumentError: missing keyword: code, given "1234" which is not a keyword argument.
```

One could argue that this situation would call for a different error class, perhaps a `KeywordArgumentError` that would inherit from `ArgumentError`, but that would extend the scope of this feature request a bit too far in my mind. 

---Files--------------------------------
missing_kwargs.diff (1.2 KB)
missing_kwargs.diff (1.23 KB)


-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-07-04  0:32 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <redmine.issue-13196.20170205201800@ruby-lang.org>
2017-02-05 20:18 ` [ruby-core:79439] [Ruby trunk Bug#13196] Improve keyword argument errors when non-keyword arguments given hi
2017-02-06  3:24 ` [ruby-core:79448] " shevegen
2017-02-06  4:09 ` [ruby-core:79451] " nobu
2017-02-09 20:00 ` [ruby-core:79488] " sto.mar
2017-02-26  3:26 ` [ruby-core:79778] " hi
2017-02-26  3:40 ` [ruby-core:79779] " hi
2017-02-26 21:07 ` [ruby-core:79791] " sto.mar
2017-02-27  1:50 ` [ruby-core:79794] " duerst
2017-03-16  8:10 ` [ruby-core:80184] " hi
2017-05-19  6:07 ` [ruby-core:81245] " matz
2017-07-04  0:31 ` [ruby-core:81890] " hi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).