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

Potential type errors #3503

Open
mckaz opened this issue Jan 15, 2020 · 2 comments
Open

Potential type errors #3503

mckaz opened this issue Jan 15, 2020 · 2 comments

Comments

@mckaz
Copy link

mckaz commented Jan 15, 2020

These were caught by a novel type inference system built on RDL. I wanted to bring these to your attention, and also confirm whether they are true bugs so that we may better evaluate our inference system.

  1. Here there is no variable or method called number.

  2. Here Billing::Integrations appears to be undefined in this scope.

@mckaz
Copy link
Author

mckaz commented Jun 22, 2020

Hi, I just wanted to follow up on these issues. The line numbers have changed since my original message, so I'll re-link below.

  1. Here there is no variable or method called number.

  2. Here Billing::Integrations appears to be undefined in this scope.

Are these true errors or were we mistaken?

@paracycle
Copy link

Hello @mckaz, thanks for reporting these issues and apologies for the super late response. We've triaged these issues and you can find our analysis below:

  1. The issue with the missing number method is due to the super relaxed way Ruby mix-ins work. While the number method is not defined on the CreditCardMethods module, the number is defined as an attr_reader in CreditCard class which mixes in this module. In summary, this usage is safe, under the assumption that CreditCardMethods will only mixed-in to CreditCard class or a class that defines a number method. Since Ruby lacks a way for modules to make that dependency explicit, it may end up looking like a bug.

    Granted, this is not the best coding practice, but it is not necessarily a bug for how it is used in this library.

  2. This was a legitimate bug, thanks for pointing it out. Billing::Integrations was removed while extracting integrations to the offsite_payments gem, but this usage (and another one in the test_helper.rb file) was left behind. We have merged a PR (Remove reference to Billing::Integrations #3692) to remove this.

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

2 participants