[ruby-core:120174] [Ruby master Bug#20943] Constant defined in `Data` block

[ruby-core:120174] [Ruby master Bug#20943] Constant defined in `Data` block

[nobu (Nobuyoshi Nakada) via ruby-core]

Issue #20943 has been reported by nobu (Nobuyoshi Nakada).

----------------------------------------
Bug #20943: Constant defined in `Data` block
https://bugs.ruby-lang.org/issues/20943

* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN
----------------------------------------
>From https://github.com/ruby/ruby/pull/12274:
> A couple times in code review I've seen constants inadvertently leak to top level from within a `Struct` or `Data` do block. I think it would be nice to show reopening the `Data` class when a constant is defined, so the constant is defined within the namespace. In this case, `Measure::NONE` instead of top level `Object::NONE`. It would also show readers that it's okay to reopen a `Data` class, which seems nice since some folk might not realize. Thanks for considering!

However, I think that `NONE` probably might be intended to be defined under `Measure`.

Current:
```ruby
Measure = Data.define(:amount, :unit) do
  NONE = Data.define
end
p NONE #=> NONE
```

Another:
```ruby
Measure = Data.define(:amount, :unit) do
  NONE = Data.define
  p NONE #=> Measure::NONE
end
p NONE # uninitialized constant NONE (NameError)
```

@zverok How do think?



-- 
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/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:120176] [Ruby master Bug#20943] Constant defined in `Data.define` block

[byroot (Jean Boussier) via ruby-core]

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


Yeah, that's a common mistake with `Struct.new` / `Data.define` / `Class.new`

That's why with `Struct` you'd often see:

```ruby
class Something < Struct.new(:a, :b)
  ...
end
```

Which is a bit wasteful as you define two classes instead of one, but not a big deal.

I kinda wish this would be valid syntax:

```ruby
class Something = Struct.new(:a, :b)
  ...
end
```

----------------------------------------
Bug #20943: Constant defined in `Data.define` block
https://bugs.ruby-lang.org/issues/20943#change-110934

* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN
----------------------------------------
>From https://github.com/ruby/ruby/pull/12274:
> A couple times in code review I've seen constants inadvertently leak to top level from within a `Struct` or `Data` do block. I think it would be nice to show reopening the `Data` class when a constant is defined, so the constant is defined within the namespace. In this case, `Measure::NONE` instead of top level `Object::NONE`. It would also show readers that it's okay to reopen a `Data` class, which seems nice since some folk might not realize. Thanks for considering!

However, I think that `NONE` probably might be intended to be defined under `Measure`.

Current:
```ruby
Measure = Data.define(:amount, :unit) do
  NONE = Data.define
end
p NONE #=> NONE
```

Another:
```ruby
Measure = Data.define(:amount, :unit) do
  NONE = Data.define
  p NONE #=> Measure::NONE
end
p NONE # uninitialized constant NONE (NameError)
```

@zverok How do think?



-- 
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/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:120182] [Ruby master Bug#20943] Constant defined in `Data.define` block

[matheusrich (Matheus Richard) via ruby-core]

Issue #20943 has been updated by matheusrich (Matheus Richard).


FYI: That also happens with `Class.new`. (maybe all blocks?)

----------------------------------------
Bug #20943: Constant defined in `Data.define` block
https://bugs.ruby-lang.org/issues/20943#change-110942

* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN
----------------------------------------
>From https://github.com/ruby/ruby/pull/12274:
> A couple times in code review I've seen constants inadvertently leak to top level from within a `Struct` or `Data` do block. I think it would be nice to show reopening the `Data` class when a constant is defined, so the constant is defined within the namespace. In this case, `Measure::NONE` instead of top level `Object::NONE`. It would also show readers that it's okay to reopen a `Data` class, which seems nice since some folk might not realize. Thanks for considering!

However, I think that `NONE` probably might be intended to be defined under `Measure`.

Current:
```ruby
Measure = Data.define(:amount, :unit) do
  NONE = Data.define
end
p NONE #=> NONE
```

Another:
```ruby
Measure = Data.define(:amount, :unit) do
  NONE = Data.define
  p NONE #=> Measure::NONE
end
p NONE # uninitialized constant NONE (NameError)
```

@zverok How do think?



-- 
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/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:120284] [Ruby master Bug#20943] Constant defined in `Data.define` block

[shan (Shannon Skipper) via ruby-core]

Issue #20943 has been updated by shan (Shannon Skipper).


byroot (Jean Boussier) wrote in #note-2:

> Which is a bit wasteful as you define two classes instead of one, but not a big deal.

I agree the extra class being created is minor, but I'm slightly more bothered by the anonymous class in the ancestry.

```ruby
[Something,
 #<Class:0x00000001438d5dd0>,
 Struct,
...
]
```
> I kinda wish this would be valid syntax:
> 
> ```ruby
> class Something = Struct.new(:a, :b)
>   ...
> end
> ```

I'd like that too. That same pattern would be great for both `Struct` and `Data` from my vantage. Or if both could keep constants in scope within `do` blocks, but would that be consider breaking? Too late for Data? I'd like it.

It seems like options include:
1. Change `Data.define do` block scope for constants
2. Add a new `Something = Data.define` or similar syntax
3. Recommend reopening classes in docs
4. Recommend inheritance in docs
5. Keep the status quo of defining a constant outside the scope in docs

That ^ list happens to be roughly in my personal order of preference from top to bottom. :) Having both 1 and 2 would also be an option.

I wonder if changing documentation to something that keeps `NONE` inside the module is worth doing in the short term? If syntax adjustments are decided against, I'd rather just recommend reopening `Data` and `Struct` classes rather than the slight back bending with existing solutions like `self::` prefix or `< Data.define`. On the other hand, I'd love to see a syntax adjustment to make it easier to define a constant within a `Data` without reopening the class.

----------------------------------------
Bug #20943: Constant defined in `Data.define` block
https://bugs.ruby-lang.org/issues/20943#change-111049

* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN
----------------------------------------
>From https://github.com/ruby/ruby/pull/12274:
> A couple times in code review I've seen constants inadvertently leak to top level from within a `Struct` or `Data` do block. I think it would be nice to show reopening the `Data` class when a constant is defined, so the constant is defined within the namespace. In this case, `Measure::NONE` instead of top level `Object::NONE`. It would also show readers that it's okay to reopen a `Data` class, which seems nice since some folk might not realize. Thanks for considering!

However, I think that `NONE` probably might be intended to be defined under `Measure`.

Current:
```ruby
Measure = Data.define(:amount, :unit) do
  NONE = Data.define
end
p NONE #=> NONE
```

Another:
```ruby
Measure = Data.define(:amount, :unit) do
  NONE = Data.define
  p NONE #=> Measure::NONE
end
p NONE # uninitialized constant NONE (NameError)
```

@zverok How do think?



-- 
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/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:120291] [Ruby master Bug#20943] Constant defined in `Data.define` block

[matz (Yukihiro Matsumoto) via ruby-core]

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


Blocks do not introduce new scope even with `instance_eval` nor `class_eval` (along with `Data.define` etc. with blocks). Changing this behavior might cause serious compatibility problems.
If someone is willing to survey and estimate how much influence it could cause, we'd like to discuss. Otherwise, it's safe to keep the behavior as it is.

Matz.


----------------------------------------
Bug #20943: Constant defined in `Data.define` block
https://bugs.ruby-lang.org/issues/20943#change-111056

* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN
----------------------------------------
>From https://github.com/ruby/ruby/pull/12274:
> A couple times in code review I've seen constants inadvertently leak to top level from within a `Struct` or `Data` do block. I think it would be nice to show reopening the `Data` class when a constant is defined, so the constant is defined within the namespace. In this case, `Measure::NONE` instead of top level `Object::NONE`. It would also show readers that it's okay to reopen a `Data` class, which seems nice since some folk might not realize. Thanks for considering!

However, I think that `NONE` probably might be intended to be defined under `Measure`.

Current:
```ruby
Measure = Data.define(:amount, :unit) do
  NONE = Data.define
end
p NONE #=> NONE
```

Another:
```ruby
Measure = Data.define(:amount, :unit) do
  NONE = Data.define
  p NONE #=> Measure::NONE
end
p NONE # uninitialized constant NONE (NameError)
```

@zverok How do think?



-- 
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/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:120293] [Ruby master Bug#20943] Constant defined in `Data.define` block

[zverok (Victor Shepelev) via ruby-core]

Issue #20943 has been updated by zverok (Victor Shepelev).


TBH, for bigger `Struct`/`Data`-based classes I typically prefer a regular inheritance instead of using class definition block—both for just aestetics (“it looks like a class definition”) and to avoid behavioral differences like constant definition discussed here, or being able to treat `Data` like a proper base class:

For example, `.new` for `Data`-based classes accepts both positional and keyword arguments. But if one is _sure_ they want to change this behavior, with regular inheritance, it is extremely easy:
```ruby
class Unit < Data.define(:value)
  def self.new(value, **nil) = super(value:)
end

Unit.new(1) #=> #<data Unit value=1>
Unit.new(value: 1)
# no keywords accepted (ArgumentError)
```
Such easy redefinition (using `super`) is not available with `Data.define(...) { ... }`.

For all I know, there are two counter-arguments for regular inheritance for those cases:
1. Creation of unnecessary anonymous immediate classes
2. Problems with code reloading

In my dayjob (large regular Rails applications) I find the former negligible, and never met with the latter, but this experience might not be universal.

Yet theoretically, I’d rather thought in the direction of changes that would make regular inheritance from `Data.define`/`Struct.new` less problematic (if it really is, and it is not just “how we are used to think of it”). 

Just to notice here: Other than Struct/Data, there are other libraries that make use of “inheriting of dynamically produced classes” approach (though I didn’t look into the implementation details for many years, so I am not sure how it is implemented currently), like [Sequel](https://sequel.jeremyevans.net/rdoc/files/README_rdoc.html#label-Sequel+Models):
```ruby
class Post < Sequel::Model(:my_posts); end
```
or, IIRC, some of the dry-rb gems.

So, it might be, that “optimizing of inheritance from dynamic classes/modules” is more desirable decision than complication of block scopes.

----------------------------------------
Bug #20943: Constant defined in `Data.define` block
https://bugs.ruby-lang.org/issues/20943#change-111058

* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN
----------------------------------------
From https://github.com/ruby/ruby/pull/12274:
> A couple times in code review I've seen constants inadvertently leak to top level from within a `Struct` or `Data` do block. I think it would be nice to show reopening the `Data` class when a constant is defined, so the constant is defined within the namespace. In this case, `Measure::NONE` instead of top level `Object::NONE`. It would also show readers that it's okay to reopen a `Data` class, which seems nice since some folk might not realize. Thanks for considering!

However, I think that `NONE` probably might be intended to be defined under `Measure`.

Current:
```ruby
Measure = Data.define(:amount, :unit) do
  NONE = Data.define
end
p NONE #=> NONE
```

Another:
```ruby
Measure = Data.define(:amount, :unit) do
  NONE = Data.define
  p NONE #=> Measure::NONE
end
p NONE # uninitialized constant NONE (NameError)
```

@zverok How do think?



-- 
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/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:120400] [Ruby master Bug#20943] Constant defined in `Data.define` block

[luke-gru (Luke Gruber) via ruby-core]

Issue #20943 has been updated by luke-gru (Luke Gruber).


You can always do this, of course:

```ruby
Measure = Data.define(:amount, :unit); class Measure
  NONE = Data.define
end
```

And I agree that `class Something = Struct.new(:a, :b)` would be nice to avoid having to do this.


----------------------------------------
Bug #20943: Constant defined in `Data.define` block
https://bugs.ruby-lang.org/issues/20943#change-111180

* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN
----------------------------------------
>From https://github.com/ruby/ruby/pull/12274:
> A couple times in code review I've seen constants inadvertently leak to top level from within a `Struct` or `Data` do block. I think it would be nice to show reopening the `Data` class when a constant is defined, so the constant is defined within the namespace. In this case, `Measure::NONE` instead of top level `Object::NONE`. It would also show readers that it's okay to reopen a `Data` class, which seems nice since some folk might not realize. Thanks for considering!

However, I think that `NONE` probably might be intended to be defined under `Measure`.

Current:
```ruby
Measure = Data.define(:amount, :unit) do
  NONE = Data.define
end
p NONE #=> NONE
```

Another:
```ruby
Measure = Data.define(:amount, :unit) do
  NONE = Data.define
  p NONE #=> Measure::NONE
end
p NONE # uninitialized constant NONE (NameError)
```

@zverok How do think?



-- 
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/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:120446] [Ruby master Bug#20943] Constant defined in `Data.define` block

[byroot (Jean Boussier) via ruby-core]

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


I opened a feature request: https://bugs.ruby-lang.org/issues/20993

----------------------------------------
Bug #20943: Constant defined in `Data.define` block
https://bugs.ruby-lang.org/issues/20943#change-111226

* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN
----------------------------------------
>From https://github.com/ruby/ruby/pull/12274:
> A couple times in code review I've seen constants inadvertently leak to top level from within a `Struct` or `Data` do block. I think it would be nice to show reopening the `Data` class when a constant is defined, so the constant is defined within the namespace. In this case, `Measure::NONE` instead of top level `Object::NONE`. It would also show readers that it's okay to reopen a `Data` class, which seems nice since some folk might not realize. Thanks for considering!

However, I think that `NONE` probably might be intended to be defined under `Measure`.

Current:
```ruby
Measure = Data.define(:amount, :unit) do
  NONE = Data.define
end
p NONE #=> NONE
```

Another:
```ruby
Measure = Data.define(:amount, :unit) do
  NONE = Data.define
  p NONE #=> Measure::NONE
end
p NONE # uninitialized constant NONE (NameError)
```

@zverok How do think?



-- 
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/lists/ruby-core.ml.ruby-lang.org/