[ruby-core:110324] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[ruby-core:110324] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[baweaver (Brandon Weaver)]

Issue #19063 has been reported by baweaver (Brandon Weaver).

----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110327] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[ioquatix (Samuel Williams)]

Issue #19063 has been updated by ioquatix (Samuel Williams).


@matz what do you think?

----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99619

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110328] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[sawa (Tsuyoshi Sawada)]

Issue #19063 has been updated by sawa (Tsuyoshi Sawada).


> When people use Hash.new([]) they mean Hash.new { |h, k| h[k] = [] }.

I don't think so. Can you prove it?

----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99620

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110329] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[baweaver (Brandon Weaver)]

Issue #19063 has been updated by baweaver (Brandon Weaver).


sawa (Tsuyoshi Sawada) wrote in #note-2:
> > When people use Hash.new([]) they mean Hash.new { |h, k| h[k] = [] }.
> 
> I don't think so. Can you prove it? For note, coming up with a lot of examples pointing towards that is not a proof.

Do you have examples of this? I have had several cases across multiple large Ruby companies where this behavior had confused people:

```ruby
3.1.0 :001 > array = []
 => []
3.1.0 :002 > hash = Hash.new(array)
 => {}
3.1.0 :003 > hash[:a] << 1
 => [1]
3.1.0 :004 > hash[:b] << 2
 => [1, 2]
3.1.0 :005 > hash[:c] << 3
 => [1, 2, 3]
3.1.0 :006 > hash
 => {}
3.1.0 :007 > array
 => [1, 2, 3]
```

It is very similar to the [default list argument in Python](https://docs.python-guide.org/writing/gotchas/) which is also confusing to people:

```python
def append_to(element, to=[]):
    to.append(element)
    return to

my_list = append_to(12)
print(my_list)
# [12]

my_other_list = append_to(42)
print(my_other_list)
# [12, 42]
```

If you would like I can poll people, but when teaching Ruby it has been a very common source of confusion and I have yet to meet someone or seen code where this was intentional. Perhaps there are, but they are very rare, and this is a very common issue to Ruby users in the places I have worked.

----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99621

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110330] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[austin (Austin Ziegler)]

Issue #19063 has been updated by austin (Austin Ziegler).


This mistake is much more common than I would have thought, and it appears in some fairly large projects (not everyone may have access to this result because `cs.github.com` is still apparently in preview):

https://cs.github.com/?scopeName=All+repos&scope=&q=language%3Aruby+Hash.new%28%5B%5D%29

> Showing 1 - 20 of about 1.5k files found (in 612 milliseconds)

These are from the first twenty instances:

- GitLab: https://github.com/gitlabhq/gitlabhq/blob/eaeb21af27304897f46f91633e25cba23232349d/lib/gitlab/api_authentication/builder.rb#L12
- FactoryBot: https://github.com/thoughtbot/factory_bot/blob/ec71611954d07419c9b668be03641332443dcd78/lib/factory_bot/linter.rb#L22
- Fat Free CRM
  - 1: https://github.com/fatfreecrm/fat_free_crm/blob/5b73c62335835106c25f547d9810197d8b4c0ba8/lib/fat_free_crm/callback.rb#L58
  - 2: https://github.com/fatfreecrm/fat_free_crm/blob/5b73c62335835106c25f547d9810197d8b4c0ba8/lib/fat_free_crm/callback.rb#L74
- Instructure Canvas LMS: https://github.com/instructure/canvas-lms/blob/8bd46cb657ec925229bfcd5407c145ec121d85c7/app/models/context.rb#L182
- Chef
  - Knife: https://github.com/chef/chef/blob/e85fcb8c0ad1ea4ed6f830e4b585f27c2ada4994/knife/lib/chef/knife.rb#L173
  - Inspec: https://github.com/inspec/inspec/blob/af5b8335af052743914909e3833a88c7ba350ed8/lib/inspec/utils/nginx_parser.rb#L93
- Redmine:
  - 1: https://github.com/redmine/redmine/blob/823080b45e58563f989b992789ed340d358ed955/app/models/user.rb#660
  - 2: https://github.com/redmine/redmine/blob/823080b45e58563f989b992789ed340d358ed955/app/models/user.rb#693
- Browser CMS: https://github.com/browsermedia/browsercms/blob/0a7fb9219f6e24cce4271b420c2ea07febd1b748/app/models/cms/category_type.rb#L21
- AWS S3 SDK (feature testing, but…)
  - 1: https://github.com/aws/aws-sdk-ruby/blob/2e96092cca28a0422fc3950f078b0919b636c227/gems/aws-sdk-s3/features/client/step_definitions.rb#344
  - 2: https://github.com/aws/aws-sdk-ruby/blob/2e96092cca28a0422fc3950f078b0919b636c227/gems/aws-sdk-s3/features/client/step_definitions.rb#366
- Barkeep: https://github.com/ooyala/barkeep/blob/126b1b1f41f514396d3e22f9935fe4640de2a402/lib/string_filter.rb#L27
- Opal: https://github.com/opal/opal/blob/934aa61bdb80a9da6ce9c92b0eedba737bbc1f9f/lib/opal/compiler.rb#L255
- OpenProject:
  - 1: https://github.com/opf/openproject/blob/56738cbfe492dcde3ab80463bff33eed7ae765f4/lib/api/v3/activities/activity_eager_loading_wrapper.rb#L76
  - 2: (`Hash.new({])`) https://github.com/opf/openproject/blob/56738cbfe492dcde3ab80463bff33eed7ae765f4/lib/api/v3/activities/activity_eager_loading_wrapper.rb#L92
- Goldiloader: https://github.com/salsify/goldiloader/blob/ead058ab65ecf845c78b51307c048a9f09f87ef7/lib/goldiloader/auto_include_context.rb#L26
- rspec-core: https://github.com/rspec/rspec-core/blob/71823ba11ec17a73b25bdc24ebab195494c270dc/lib/rspec/core/filter_manager.rb#L205-L206
- @shugo's textbringer: https://github.com/shugo/textbringer/blob/65dff909295f14f466f7b27e824ea36fe1f9ddd7/bin/merge_mazegaki_dic#L5

So this is *very* widespread, and experienced Rubyists make this mistake.

Because this is something that would only affect Ruby versions going forward, I think that updating Rubocop and other linters is a necessary *first* step. Of the rest of the proposals, I think that in the short to medium term, the only viable option is _2. Warn About Unexpected Behavior_. I think that the *least* viable is _1. Do What They Meant_, and the better long time option is _3. Require Frozen or Values_.

----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99622

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110331] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[baweaver (Brandon Weaver)]

Issue #19063 has been updated by baweaver (Brandon Weaver).


> For note, coming up with a lot of examples pointing towards that is not a proof.

Then what would qualify as a proof? I am confused by this as it asserts that examples of people being confused are not proof, whenever by definition it proves that people are confused by this even if it does work as intended.

In Python there is the `exit()` syntax, and when one uses `exit` instead you get this:

```python
>>> exit
Use exit() or Ctrl-D (i.e. EOF) to exit
```

One can argue that this works as intended, but the impact is that many people are still confused by this. We must not forget the impact of language choices, despite the intention behind them, as they make our language more unapproachable.

----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99623

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110332] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[baweaver (Brandon Weaver)]

Issue #19063 has been updated by baweaver (Brandon Weaver).


> austin (Austin Ziegler) wrote in #note-4:
> So this is *very* widespread, and experienced Rubyists make this mistake.
> 
> Because this is something that would only affect Ruby versions going forward, I think that updating Rubocop and other linters is a necessary *first* step. Of the rest of the proposals, I think that in the short to medium term, the only viable option is _2. Warn About Unexpected Behavior_. I think that the *least* viable is _1. Do What They Meant_, and the better long time option is _3. Require Frozen or Values_.

I would agree that option 2 is the most viable without serious breaking behavior, and option 3 would be better long-term. Option 1 was an attempt at a compromise, but would have breaking changes as well.


----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99624

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110333] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[austin (Austin Ziegler)]

Issue #19063 has been updated by austin (Austin Ziegler).


sawa (Tsuyoshi Sawada) wrote in #note-2:
> > When people use Hash.new([]) they mean Hash.new { |h, k| h[k] = [] }.
> I don't think so. Can you prove it? For note, coming up with a lot of examples pointing towards that is not a proof.

It partially comes from not necessarily understanding `default_value` in the Hash documentation. The documentation
only uses value objects, not mutable objects, which leaves opportunities for this to be a mistake.

It may not matter in short-lived scripts where the amount of bucketing in the Hash is small, but anything larger or longer-lived is likely to see unexpected behaviour. In the examples that I found on GitHub, none of the places where this was used seemed to be *intending* this to result in a single value, and none of them were doing `hash[key] = []` explicitly to override the use of the default value.

> >  it is a known bug
> This contradicts with what you have admitted:
> > @hsbt (Hiroshi SHIBATA) has said in the past, this is behaving as intended

This is not a contradiction. Every time I have seen this used, it is a *bug* in the system where the code was written.

It’s not a bug in Ruby…but perhaps it *should* be. I’m in favour of making it a warning, but it should be easy to turn off for the rare case when it is intended.

----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99625

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110334] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[austin (Austin Ziegler)]

Issue #19063 has been updated by austin (Austin Ziegler).


Edit on a couple of these. The GitLab behaviour, and @shugo’s code both do `hash[key] |= value_list`, so they *do* effectively do `hash[key] = value_list`, which makes them safer to use *in these particular cases*. I don’t think that this would necessarily be the case for every instance. I also happen to think that the use of `hash[key] |= value_list` is *clever* code, which is more likely to indicate mistakes.

I think that warning on the direct use of `[]`, `{}`, and `""` literals is a good choice (call that proposal 2.1), but *not* warning on the use of a variable.

----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99626

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110335] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[sawa (Tsuyoshi Sawada)]

Issue #19063 has been updated by sawa (Tsuyoshi Sawada).


baweaver (Brandon Weaver) wrote in #note-5:
> Then what would qualify as a proof?

I don't think it is provable. That is my point. You made a statement that cannot be assured.

I know that a lot of beginners make the kind of mistake you mention. And I am not necessarily against improving that situation. However, I am against your description of the issue from a few respects.

1. Your assertion of this issue as being Ruby's bug or the behavior as unintended is a mistake as you have admitted in comment #3, and I consider it hostile to the Ruby developers. At most, you can say that it is a pitfall, but never as unintended. Code written by a beginner using this feature may likely (but not necessarily) be unintended, but the Ruby's behavior is not unintended. You should not confuse them.
2. Your proposal 1 would break one of the very basic principles of Ruby, or perhaps of any computer language, or even any natural language. That is, the arguments passed to a method are evaluated before (or independently of) the execution of the method. In other words, the semantics of the whole is calculated from the semantics of its parts. In the context of natural language, this principle is known as Frege's compositionality.

Only proposal 2 seems to make sense to me, but with qualification that "it is a known bug" be removed.


----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99627

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110336] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[ioquatix (Samuel Williams)]

Issue #19063 has been updated by ioquatix (Samuel Williams).


The current behaviour is ambiguous at best and as @austin has shown, buggy and incorrect at worst / in practice.

I prefer option 1 but it's true it can break compatibility with this poorly understood behaviour.

I agree in principle to issue a warning but I also don't think we should prevent the current usage, so I propose changing the interface slightly:

```ruby
Hash.new(default_value, assign: true/false)
```

The current behaviour is retained by `assign: false`. Setting `assign: true` gives the proposal 1 behaviour, calling `default_value.dup`.

Then, we can warn if `assign` is not specified.




----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99628

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110337] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[duerst]

Issue #19063 has been updated by duerst (Martin Dürst).


sawa (Tsuyoshi Sawada) wrote in #note-9:

> 2. Your proposal 1 would break one of the very basic principles of Ruby, or perhaps of any computer language, or even any natural language. That is, the arguments passed to a method are evaluated before (or independently of) the execution of the method. In other words, the semantics of the whole is calculated from the semantics of its parts. In the context of natural language, this principle is known as Frege's compositionality. What is relevant here is that the default object that is passed as an argument must be evaluated to  be such object before initialization of the hash. A block does not have such limitation because it is not a default object per se, but is something that would be evaluated to be a default object.

There would definitely be quite a few details to be worked out, but starting with `Hash.new([])`, I don't see any problem. One of the basic principles of method arguments is that methods can respond differently to different arguments. That's the whole point of arguments. Of course, `Hash.new(my_method_returning_an_empty_array)` would behave exactly the same as `Hash.new([])` (assuming `my_method_returning_an_empty_array`) really did what its name said). So compositionality would still hold.



----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99629

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110338] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[duerst]

Issue #19063 has been updated by duerst (Martin Dürst).


ioquatix (Samuel Williams) wrote in #note-10:
> The current behaviour is ambiguous at best and as @austin has shown, buggy and incorrect at worst / in practice.
> 
> I prefer option 1 but it's true it can break compatibility with this poorly understood behaviour.

There is definitely a chance of breaking compatibility. It would be good to find actual examples where `Hash.new([])` (or anything similar to it) was intended as specified by Ruby. I haven't seen any such examples yet.

> I agree in principle to issue a warning but I also don't think we should prevent the current usage, so I propose changing the interface slightly:
> 
> ```ruby
> Hash.new(default_value, assign: true/false)
> ```
> 
> The current behaviour is retained by `assign: false`. Setting `assign: true` gives the proposal 1 behaviour, calling `default_value.dup`.
> 
> Then, we can warn if `assign` is not specified.

I think `assign` is ambiguous, as in any case something gets assigned somehow. I'd prefer something like `dup: true/false`, or maybe even `Hash.new(dup: [])`. The later would be a shorthand for `Hash.new { |h,k| h[k] = [] }`. But that wouldn't eliminate the potential for misunderstandings for `Hash.new([])`. 

----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99630

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110339] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[sawa (Tsuyoshi Sawada)]

Issue #19063 has been updated by sawa (Tsuyoshi Sawada).


duerst (Martin Dürst) wrote in #note-11:

> `Hash.new(my_method_returning_an_empty_array)` would behave exactly the same as `Hash.new([])` (assuming `my_method_returning_an_empty_array`) really did what its name said). So compositionality would still hold.

There is no problem with whatever `my_method_returning_an_empty_array` is as long as it is defined independent of its caller `Hash.new`. Proposal 1 is suggesting to let `Hash.new` control what `my_method_returning_an_empty_array` refers to. That breaks compositionality. Or else, it is suggesting to let the argument provide something like "a prototype of the default object" instead of the default object itself. That would change the whole role of the argument for this method.

----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99631

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110340] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[sawa (Tsuyoshi Sawada)]

Issue #19063 has been updated by sawa (Tsuyoshi Sawada).


This issue is essentially the same issue as people mistakenly writing `Array.new(5, [])` with the intention to actually do `Array.new(5){[]}`.

Users need to have better understanding of how argument passing works. Better documentation and warning may be a good idea. An ad hoc modification to the feature as in Proposal 1 would not really solve the problem.

----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99632

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110343] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[sawa (Tsuyoshi Sawada)]

Issue #19063 has been updated by sawa (Tsuyoshi Sawada).


I can imagine why you particularly picked up (or people are more troubled with) `Hash.new` rather than `Array.new`. Probably, you are feeling that `Hash.new {|h, k| h[k] = []}` is too long to write. So I have an even better idea. If possible, what about actually allowing:

```ruby
Hash.new{[]}
```

to be equivalent to:

```ruby
Hash.new{|h, k| h[k] = []}
```

paying attention to the block variable signature? When the block signature is absent, the return value of the block would be automatically assigned. This would mean something like this:

```ruby
h1 = Hash.new{|h, k| h[k] = "foo"}
h2 = Hash.new{|h, k| "foo"}
h3 = Hash.new{"foo"}

h1[:a] # => "foo"
h2[:a] # => "foo"
h3[:a] # => "foo"

h1 # => {:a=>"foo"}
h2 # => {}
h3 # => {:a=>"foo"}
```

----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99634

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110345] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[ioquatix (Samuel Williams)]

Issue #19063 has been updated by ioquatix (Samuel Williams).


@sawa I appreciate your technical knowledge and insight.

However... the practical reality is the vast majority of the users are using this interface incorrectly. This might imply that the theoretical basis for how `Hash.new(default_value)` and `Array.new(count, default_value)` work, is impractical and not what users expect.

There are different ways you can frame this, all equally valid from a technical point of view. However, the current framing of the problem is causing real problems in production code.

So we need to propose how to

1. introduce a safer interface (`dup` or `assign` style options, `{}` constructor with 0-arity), and
2. inform existing users of the problem (warning, deprecation, etc),

Your proposal can help with (1), but I'm not sure it can help with (2). The question is, are there valid use cases for `Hash.new(default_value)` and/or `Array.new(count, default_value)` where the user expects mutable values to be shared? I have not yet seen a single compelling use case.

----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99636

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110347] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[ioquatix (Samuel Williams)]

Issue #19063 has been updated by ioquatix (Samuel Williams).


In support of `Hash.new{default_value}`, `Array.new(count){default_value}` works in the same way.

----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99638

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110364] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[Dan0042 (Daniel DeLorme)]

Issue #19063 has been updated by Dan0042 (Daniel DeLorme).


This is a good idea. Even though the current behavior is "logical", and experienced rubyists have gotten so used to this that it's second nature, there's an opportunity to make ruby more friendly for beginners for removing a well-known gotcha.

Proposal 1 is just too magical for me. Using an empty array would mean something special and different from everything else.

Proposal 2 would cause warnings for valid code such as `h[1] += [2]`. It's good to help future rubyists write future code, but we have to consider existing code that is not broken. It's not good to force a change on valid code just because "this is the new way to do it".

Proposal 3 is imho the best. Existing code like `h[1] += [2]` would still work. It would cause code like `h[1] << 2` to raise an exception, but that code is *already* broken anyway. So the backward compatibility issue is minor I think. But this could be a good opportunity to introduce #16153 as a way to have a deprecation phase for this change. So the previous code could warn "[] will be eventually frozen" instead of raising an exception.


----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99657

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110365] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[Dan0042 (Daniel DeLorme)]

Issue #19063 has been updated by Dan0042 (Daniel DeLorme).


sawa (Tsuyoshi Sawada) wrote in #note-15:
> ```ruby
> h2 = Hash.new{|h, k| "foo"}
> h3 = Hash.new{"foo"}
> 
> h2[:a] # => "foo"
> h3[:a] # => "foo"
> 
> h2 # => {}
> h3 # => {:a=>"foo"}
> ```

I *love* this idea! It makes the common case so much easier and uncluttered. It's easy to bypass if you need to. It works great in combination with a frozen default value; when `Hash.new([])` doesn't work because you can't append to a frozen value, you then reach for the next option `Hash.new{[]}`

The main downside is incompatibility with previous ruby versions. If this change makes it into 3.2 and you use that version when writing `Hash.new{[]}`, then someone else running that code with ruby 3.1 will get buggy hard-to-diagnose behavior instead of a simple exception. So maybe this would be better as a new method, like `Hash.new!{[]}` or `Hash{[]}`.

Note that I haven't found that many gems that use `Hash.new{value}` syntax, and most of them would work fine with this change:

```ruby
#actionpack-7.0.3/lib/action_controller/metal/parameter_encoding.rb
49:        @_parameter_encodings[action.to_s] = Hash.new { Encoding::ASCII_8BIT }

#actionview-7.0.3/lib/action_view/helpers/tag_helper.rb
40:      PRE_CONTENT_STRINGS             = Hash.new { "" }

#bunny-2.19.0/lib/bunny/authentication/credentials_encoder.rb
29:        @@registry ||= Hash.new { raise NotImplementedError }

#cucumber-core-10.1.1/lib/cucumber/core/test/filters/tag_filter.rb
32:            @test_cases_by_tag_name = Hash.new { [] }

#cucumber-core-10.1.1/lib/cucumber/core/test/result.rb
340:            @totals = Hash.new { 0 }

#cucumber-core-11.0.0/lib/cucumber/core/test/filters/tag_filter.rb
32:            @test_cases_by_tag_name = Hash.new { [] }

#cucumber-core-11.0.0/lib/cucumber/core/test/result.rb
340:            @totals = Hash.new { 0 }

#ice_nine-0.11.2/spec/unit/ice_nine/freezer/hash/class_methods/deep_freeze_spec.rb
13:      Hash.new {}.update(Object.new => Object.new)

#resque-2.2.1/lib/resque/server.rb
110:        hosts = Hash.new { [] }

#rspec-core-3.11.0/lib/rspec/core/formatters/deprecation_formatter.rb
144:            @seen_deprecations = Hash.new { 0 }

#rspec-core-3.11.0/lib/rspec/core/hooks.rb
496:          :before => Hash.new { BeforeHook },
497:          :after  => Hash.new { AfterHook  },
498:          :around => Hash.new { AroundHook }

#sequel-5.57.0/lib/sequel/extensions/_pretty_table.rb
42:      sizes = Hash.new {0}

#sprockets-4.1.1/lib/sprockets/transformers.rb
150:        transformers          = Hash.new { {} }
151:        inverted_transformers = Hash.new { Set.new }
```


----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99658

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110366] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[sawa (Tsuyoshi Sawada)]

Issue #19063 has been updated by sawa (Tsuyoshi Sawada).


Dan0042 (Daniel DeLorme) wrote in #note-19:
> The main downside is incompatibility with previous ruby versions. [...] Note that I haven't found that many gems that use `Hash.new{value}` syntax, and most of them would work fine with this change

Right. If it causes any problems, a redundant `|h, k|` would need to be added to such cases. But I think that transition would not be so difficult.

----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99660

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110367] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[jeremyevans0 (Jeremy Evans)]

Issue #19063 has been updated by jeremyevans0 (Jeremy Evans).


ioquatix (Samuel Williams) wrote in #note-16:
> The question is, are there valid use cases for `Hash.new(default_value)` and/or `Array.new(count, default_value)` where the user expects mutable values to be shared?

Sure:

```ruby
class A; end
class B < A; end
class C < A; end
class_map = Hash.new(A)
class_map[:b] = B
class_map[:c] = C
```

Just because the default value is mutable doesn't mean the user's code plans to mutate it.

----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99661

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110370] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[baweaver (Brandon Weaver)]

Issue #19063 has been updated by baweaver (Brandon Weaver).


I would agree that `Hash.new { [] }` would be very nice, and as mentioned has a dual in `Array.new(5) { [] }`.

While I do not think that it is a bug (and my apologies for earlier bad wording) to have `Hash.new([])` do what it currently does, I would like to clarify what I meant. My intended meaning was that new Ruby users _think_ it is a bug when they realize the behavior, which can be the cause of confusion. My desire is to make Ruby more understandable for new users, and prevent confusion where possible.

I appreciate that newer versions of the docs explicitly call this behavior out, I had not seen that in the past, so I would like to thank whoever added that.

Given the above conversation I would still be in favor of a warning on instances of `Hash.new` when called with mutable objects like `[]` as this does a few things:

1. Call out potentially confusing behavior for new users
2. Not break compatibility
3. Could be turned off by advanced users who intend this behavior

Outside of clever usages in experimental repos I have not been finding intentional usages of this syntax, so I would think this would be an acceptable compromise given the frequency of unintended usage as noted by @austin.


@sawa / @matz - Should we open a new ticket for this shorthand `Hash.new { [] }` syntax? It may be worth its own new conversation, but I am very positive of this syntax.

----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99664

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110372] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[sawa (Tsuyoshi Sawada)]

Issue #19063 has been updated by sawa (Tsuyoshi Sawada).


@baweaver (Brandon Weaver) Either way is okay with me.

----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99666

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110388] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[etienne]

Issue #19063 has been updated by etienne (Étienne Barrié).


While I like the idea of checking the arity of the block, maybe the core issue isn't entirely with the API for `Hash.new`, but the fact that default value/default proc are not visible. From the examples above, when we print the value of hash, it appears empty: `{}`.

By tweaking what is displayed by `Hash#inspect` and `Hash#pretty_print`, I believe it would be clearer to beginners and experienced Rubyists alike:

```ruby
array = []
# => []
hash = Hash.new(array)
# => {}
hash[:a] << 1
# => [1]
hash[:b] << 2
# => [1, 2]
hash[:c] << 3
# => [1, 2, 3]
hash
# => #<Hash default=[1, 2, 3] {}>
```


----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99694

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110393] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[byroot (Jean Boussier)]

Issue #19063 has been updated by byroot (Jean Boussier).


I'm also of the stance that this shouldn't surprise someone familiar with how Ruby is evaluated, but it is true that people have to start from somewhere and it's always good to try to help them get that understanding.

The problem with warning on mutable `default` is:

  - As mentioned before, yes mutable default is rare, but no unimaginable. It's a tough sell for a programming language to ban some usage like this.
  - Few people run Ruby in verbose mode, and certainly not the beginners for whom this is intended.
  - There's no convenient way to turn of a specific warning, short of adding another parameter or something.

That's why maybe this makes more sense as a rubocop cop.

That said, `Hash.new { [] }` would be a very welcome addition though, and is easy to implement. I agree you should open a feature request for it.

----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99699

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110402] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[sawa (Tsuyoshi Sawada)]

Issue #19063 has been updated by sawa (Tsuyoshi Sawada).


I opened a new issue #19069 regarding my proposal https://bugs.ruby-lang.org/issues/19063#note-15. If you have comments related to that idea, please continue on #19069.

----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99711

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110412] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[nobu (Nobuyoshi Nakada)]

Issue #19063 has been updated by nobu (Nobuyoshi Nakada).


Adding a new element in the default proc will add by just referencing.
I'm not for doing it implicitly.

----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99724

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110413] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[nobu (Nobuyoshi Nakada)]

Issue #19063 has been updated by nobu (Nobuyoshi Nakada).


austin (Austin Ziegler) wrote in #note-4:
> **Note**: At least two of the examples later use `hash[key] |= values`, which is assignment, not resulting in the possible reuse of the default value in general. Others may result in the same.

Another example in enc/make_encmake.rb:
```ruby
  deps = Hash.new {[]}

  # ...
        deps[e] <<= n unless default_deps.include?(n)
```

----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99725

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110414] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[baweaver (Brandon Weaver)]

Issue #19063 has been updated by baweaver (Brandon Weaver).


etienne (Étienne Barrié) wrote in #note-24:
> While I like the idea of checking the arity of the block, maybe the core issue isn't entirely with the API for `Hash.new`, but the fact that default value/default proc are not visible. From the examples above, when we print the value of hash, it appears empty: `{}`.
> 
> By tweaking what is displayed by `Hash#inspect` and `Hash#pretty_print`, I believe it would be clearer to beginners and experienced Rubyists alike:
> 
> ```ruby
> array = []
> # => []
> hash = Hash.new(array)
> # => {}
> hash[:a] << 1
> # => [1]
> hash[:b] << 2
> # => [1, 2]
> hash[:c] << 3
> # => [1, 2, 3]
> hash
> # => #<Hash default=[1, 2, 3] {}>
> ```

I would also be very positive of this change, potentially for Array as well, to make this visible to folks working in the language. What would be the impact of changing `inspect` in this way? I do not think it would be breaking, but would want others opinions on this.

----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99726

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110420] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[Eregon (Benoit Daloze)]

Issue #19063 has been updated by Eregon (Benoit Daloze).


baweaver (Brandon Weaver) wrote in #note-29:
> I would also be very positive of this change, potentially for Array as well, to make this visible to folks working in the language.

For Array it doesn't make sense since the default value or block is evaluated only upon construction, and there is no need to keep it after.

> What would be the impact of changing `inspect` in this way? I do not think it would be breaking, but would want others opinions on this.

I'd think it'd break many programs which currently assume `eval hash.inspect` works, plus surprise people.

Based on Jeremy's example in https://bugs.ruby-lang.org/issues/19063#note-21 I think there is nothing we can do here, there are valid usages of a mutable default value.
It should be clear to any most programmers that `Hash.new(...)` only evaluates the value once (unlike `Hash.new { ... }`, and whether it `.dup` or so on each access is documented (it doesn't `.dup`).

----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99731

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110424] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[shugo (Shugo Maeda)]

Issue #19063 has been updated by shugo (Shugo Maeda).


austin (Austin Ziegler) wrote in #note-8:
> Edit on a couple of these. The GitLab behaviour, and @shugo’s code both do `hash[key] |= value_list`, so they *do* effectively do `hash[key] = value_list`, which makes them safer to use *in these particular cases*. I don’t think that this would necessarily be the case for every instance. I also happen to think that the use of `hash[key] |= value_list` is *clever* code, which is more likely to indicate mistakes.
> 
> I think that warning on the direct use of `[]`, `{}`, and `""` literals is a good choice (call that proposal 2.1), but *not* warning on the use of a variable. Maybe, as well, it only warns on the direct use of the *empty* literals.

`hash[key] |= value_list` is trivial code for those who understand side effects.
I'm against for introducing such false positive warnings.


----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99734

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110425] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[mame (Yusuke Endoh)]

Issue #19063 has been updated by mame (Yusuke Endoh).


Summary so far

### Is there any example of `Hash.new([])` being used intentionally?

Yes. [Shugo's example](https://github.com/shugo/textbringer/blob/65dff909295f14f466f7b27e824ea36fe1f9ddd7/bin/merge_mazegaki_dic#L5) is a valid usage of `Hash.new([])`.

### Is there any example of `Hash.new(mutable object)` being used intentionally?

Yes. [Jeremy's example](https://bugs.ruby-lang.org/issues/19063#note-21) is a valid usage of a mutable default value.

### What will Proposal 1 break?

Jeremy's example can lead to strange situation because class objects are dup'ed.

### What will Proposal 2 break?

Shugo's example will be warned nevertheless it is legitimate.

### What will Proposal 3 break?

Both Shugo's example and Jeremy's example will require modification.

---

Next, we have a question: is this change worth the pain of incompatibility? I think people have different opinions.

Here's my personal opinion, however, I don't think we need to discuss this if we are going to add a cop to Rubocop. Rubocop is already known for excessive warnings, and provides a mechanism to selectively suppress warnings. So I agree with @byroot: it would make more sense as a Rubocop cop.

----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99735

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:110438] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[shugo (Shugo Maeda)]

Issue #19063 has been updated by shugo (Shugo Maeda).


austin (Austin Ziegler) wrote in #note-4:
> This mistake is much more common than I would have thought, and it appears in some fairly large projects (not everyone may have access to this result because cs.github.com is still apparently in preview):

It seems that only AWS S3 SDK has invalid code except when their clients mutate Hash values.

> - FactoryBot: https://github.com/thoughtbot/factory_bot/blob/ec71611954d07419c9b668be03641332443dcd78/lib/factory_bot/linter.rb#L22

valid code: `result[factory] |= errors`

> - Fat Free CRM
>   - 1: https://github.com/fatfreecrm/fat_free_crm/blob/5b73c62335835106c25f547d9810197d8b4c0ba8/lib/fat_free_crm/callback.rb#L58

valid code: `response[op[:position]] += [op[:proc].call(caller, context)]`

>   - 2: https://github.com/fatfreecrm/fat_free_crm/blob/5b73c62335835106c25f547d9810197d8b4c0ba8/lib/fat_free_crm/callback.rb#L74

valid code: `@view_hooks[hook] += [{ proc: proc,`

> - Instructure Canvas LMS: https://github.com/instructure/canvas-lms/blob/8bd46cb657ec925229bfcd5407c145ec121d85c7/app/models/context.rb#L182

valid code: `ids_by_type[type] += [id]`

> - Chef
>   - Knife: https://github.com/chef/chef/blob/e85fcb8c0ad1ea4ed6f830e4b585f27c2ada4994/knife/lib/chef/knife.rb#L173

valid code: `ids_by_type[type] += [id]`

>   - Inspec: https://github.com/inspec/inspec/blob/af5b8335af052743914909e3833a88c7ba350ed8/lib/inspec/utils/nginx_parser.rb#L93

valid code: `agg_conf[x.key] += [x.vals]`

> - Redmine:
>   - 1: https://github.com/redmine/redmine/blob/823080b45e58563f989b992789ed340d358ed955/app/models/user.rb#660

valid code `result[role] = Project.where(:id => ids).to_a`

>   - 2: https://github.com/redmine/redmine/blob/823080b45e58563f989b992789ed340d358ed955/app/models/user.rb#693

valid code: `result[role] = Project.where(:id => ids).to_a`

> - Browser CMS: https://github.com/browsermedia/browsercms/blob/0a7fb9219f6e24cce4271b420c2ea07febd1b748/app/models/cms/category_type.rb#L21

valid code: `map[ct.id.to_s] = ct.category_list.map { |c| [c.path, c.id.to_s] `

> - AWS S3 SDK (feature testing, but…)
>   - 1: https://github.com/aws/aws-sdk-ruby/blob/2e96092cca28a0422fc3950f078b0919b636c227/gems/aws-sdk-s3/features/client/step_definitions.rb#344

invalid code: `@tracker[type.to_sym] << event`

>   - 2: https://github.com/aws/aws-sdk-ruby/blob/2e96092cca28a0422fc3950f078b0919b636c227/gems/aws-sdk-s3/features/client/step_definitions.rb#366

invalid code: `@tracker[type.to_sym] << event`

> - Barkeep: https://github.com/ooyala/barkeep/blob/126b1b1f41f514396d3e22f9935fe4640de2a402/lib/string_filter.rb#L27

valid code: `filter_methods.merge!({ method_name => filter_methods[method_name] + [filter_block] })`

> - Opal: https://github.com/opal/opal/blob/934aa61bdb80a9da6ce9c92b0eedba737bbc1f9f/lib/opal/compiler.rb#L255

valid code: `@comments = ::Parser::Source::Comment.associate_locations(first_node, comments)`

> - OpenProject:
>   - 1: https://github.com/opf/openproject/blob/56738cbfe492dcde3ab80463bff33eed7ae765f4/lib/api/v3/activities/activity_eager_loading_wrapper.rb#L76

valid code: `hash[class_name] = class_name`

>   - 2: (`Hash.new({})`) https://github.com/opf/openproject/blob/56738cbfe492dcde3ab80463bff33eed7ae765f4/lib/api/v3/activities/activity_eager_loading_wrapper.rb#L92

valid code: `hash[class_name] = class_name`

> - Goldiloader: https://github.com/salsify/goldiloader/blob/ead058ab65ecf845c78b51307c048a9f09f87ef7/lib/goldiloader/auto_include_context.rb#L26

valid code: `register_models(nested_models, nested_associations[association])`

> - rspec-core: https://github.com/rspec/rspec-core/blob/71823ba11ec17a73b25bdc24ebab195494c270dc/lib/rspec/core/filter_manager.rb#L205-L206

valid code:
```
        no_location_filters = locations[
          File.expand_path(ex_metadata[:rerun_file_path])
        ].empty?
```


----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-99752

* Author: baweaver (Brandon Weaver)
* Status: Open
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



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

[ruby-core:111304] [Ruby master Feature#19063] Hash.new with non-value objects should be less confusing

[matz (Yukihiro Matsumoto)]

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

Status changed from Open to Rejected

Sorry for being late to reply. As @shugo mentioned above, there are a lot of code that use this functionality valid.
Considering this fact, we are not going to add any warning here for the core.

Matz.


----------------------------------------
Feature #19063: Hash.new with non-value objects should be less confusing
https://bugs.ruby-lang.org/issues/19063#change-100671

* Author: baweaver (Brandon Weaver)
* Status: Rejected
* Priority: Normal
----------------------------------------
Related to #10713 and #2764.

Ruby's `Hash.new` accepts either a block or a param for its default value. In the case of non-value objects this leads to unexpected behaviors:

```ruby
bad_hash_with_array_values = Hash.new([])
good_hash_with_array_values = Hash.new { |h, k| h[k] = [] }
```

While, as @hsbt has said in the past, this is behaving as intended for the Ruby language it has caused a lot of confusion in the community over the years and is a known sharp-edge.

My assertion is that this is not the intended behavior, and I cannot find a legitimate usecase in which someone intends for this to happen. More often new users to Ruby are confused by this behavior and spend a lot of time debugging.

We must consider the impact to Ruby users, despite what the intent of the language is, and make the language more clear where possible.

Given that, I have a few potential proposals for Ruby committers.

### Proposal 1: Do What They Meant

When people use `Hash.new([])` they mean `Hash.new { |h, k| h[k] = [] }`. Can we make that the case that if you pass a mutable or non-value object that the behavior will be as intended using `dup` or other techniques?

When used in the above incorrect way it is likely if not always broken code.

### Proposal 2: Warn About Unexpected Behavior

As mentioned above, I do not believe there are legitimate usages of `Hash.new([])`, and it is a known bug to many users as they do not intend for that behavior. It may be worthwhile to warn people if they do use it.

### Proposal 3: Require Frozen or Values

This is more breaking than the above, but it may make sense to require any value passed to `Hash.new` to either be `frozen` or a value object (e.g. `1` or `true`)

## Updating RuboCop

Failing this, I am considering advocating for RuboCop and similar linters to warn people against this behavior as it is not intended in most to all cases:

https://github.com/rubocop/rubocop/issues/11013

...but as @ioquatix has mentioned on the issue it would make more sense to fix Ruby rather than put a patch on top of it. I would be inclined to agree with his assessment, and would rather fix this at a language level as it is a known point of confusion.

## Final Thoughts

I would ask that maintainers consider the confusion that this has caused in the community, rather than asserting this "works as intended." It does work as intended, but the intended functionality can make Ruby more difficult for beginners. We should keep this in mind.



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/