rspec / rspec-core

RSpec runner and formatters
http://rspec.info
MIT License
1.23k stars 765 forks source link

Guard against name collisions with internals #3063

Closed ksol closed 3 days ago

ksol commented 10 months ago

Subject of the issue

Hello,

I hope this isn't a topic that was discussed already, I couldn't find a similar ticket but the search terms are pretty generic.

A colleague spent some time trying to figure out the source of an error, and it was due to a name collision. We had a let(:app) in a request spec, and it turns out app is needed by the internals of the request spec.

I caught it quickly because I add a similar issue in the past, although I'm unsure which name it was.

In any case, would it be doable and desired to guard against collisions? It could be a warning or an error (the latter would be better imo).

If not doable or not desired, would it be at least possible to get a list of which names should not be overriden (via let or other methods) ?

Thanks for your time and for a great project!

JonRowe commented 10 months ago

Its hard to guard against collisions because its "a feature not a bug" your supposed to be able to define overridden lets and they are just methods,

In the case of app its not even something rspec-core knows about, its an rspec-rails extension following a Rack convention, in fact if you're using rack-test its actually a requiement to have and testing rack defining app via let is supported and used in the wild; so its not always a collision.

To build this functionality someone would have to either do something like build a registry of conflicting names to check with an api to extend this list from other libraries / example groups, or modify the let functionality to check for existing methods and cross reference against the list of allowed overrides, probably with configurable behaviour because people.

pirj commented 10 months ago

Thanks for reporting! This is what I could find:

JonRowe commented 3 days ago

Closing as not planned during the monorepo migration, but if someone wanted to work on something for registering reserved names...