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/

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/

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+Mode...): ```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/

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/