Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Guard against name collisions with internals #3063

Open
ksol opened this issue Jan 11, 2024 · 2 comments
Open

Guard against name collisions with internals #3063

ksol opened this issue Jan 11, 2024 · 2 comments

Comments

@ksol
Copy link

ksol commented Jan 11, 2024

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!

@ksol ksol changed the title Guard against name collisions Guard against name collisions with internals Jan 11, 2024
@JonRowe
Copy link
Member

JonRowe commented Jan 11, 2024

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
Copy link
Member

pirj commented Jan 11, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants