Add rule about not extending Data.define

[ktopolski]

See also:

• PR adding new cop for this rule
• Original issue

I was wondering about two things in this PR:

1. Whether I should place it close to [[no-extend-struct-new]] rule. Decided to just append to the end of the guide eventually.
2. Whether or not I should change the [[struct-new]] rule to using Data.define instead. Kinda depends on the use case, but I'd assume Data.define should be the default for rubies 3.2+ 🤔 Looking forward to the discussion here, I'm open to adding this change to this PR(or opening new PR for that)

[pirj]

should place it close to [[no-extend-struct-new]] rule.

I'd say yes.

should change the [[struct-new]] rule to using Data.define instead

In separate PR - yes. Let's keep the Struct.new there, since it's still the only available option for most.

[pirj]

introduce weird errors if the file is required multiple times

I don't really get this part.

class A < Data.define(:x); end
class A < Data.define(:x); end # => TypeError: superclass mismatch for class A
class B < Struct.new(:x); end
class B < Struct.new(:x); end # => TypeError: superclass mismatch for class B

Also, require docs:

Loads the given name, returning true if successful and false if the feature is already loaded.

[ktopolski]

Nice catch! I think I must've forgot to remove the part about introduce weird errors... when I copied and adjusted the description from the [[no-extend-struct-new]].

I guess now the question is - what did the author of [[no-extend-struct-new]] meant there by weird errors and if it's still relevant 🤔

I'll remove the part after and, thank you!

[pirj]

define(name, *symbols) → class
Defines a new Data class. If the first argument is a string, the class is stored in Data::<name> constant.

Does it really? cc @zverok

E = Data.define('Foo', :x)
Data::Foo # => NameError: uninitialized constant Data::Foo

[ktopolski]

I'd say yes.

Alright, changed the order so [[no-extend-data-define]] appears straight after [[no-extend-struct-new]] ✅

In separate PR - yes. Let's keep the Struct.new there, since it's still the only available option for most.

I don't get this part, sorry! Got confused hwew:

In separate PR - yes. Let's keep the Struct.new there

By there you mean this PR, or in general the [[struct-new]] rule? 🤔 I'm not sure if I should open another PR to change [[struct-new]] to use Data(as you mentioned - that would be for minorities) or open another PR to add the [[data-new]] rule, or something else is suggested? 😅

[bbatsov]

I think it will also be a good idea to show the output of Person.ancestors in both examples to show clearly the anonymous class that gets added there.

[ktopolski]

Done! Do you think I should open another one to illustrate this for [[no-extend-struct-new]] too?

[zverok]

Does it really? cc @zverok

It does not. It did in initial implementation (for feature parity with Struct), but other core devs suggested that this function for Struct was legacy and there is no value in reproducing it in Data

[zverok]

NB: I am not sure how valuable this input is, but there are cases when classic inheritance from Data.define is useful: see my blogpost about initialization, "You need to be mindful redefining #initialize" section (classic subclassing allows to use super in redefined new)