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
~~Consider disabling~~ Re-enable Lint/MissingSuper #195
Comments
Thanks for calling this out! Enough people use sorbet that I think this alone is worth rolling back. Even as I added it I could imagine a couple corner cases where not calling super was the right thing to do |
Landed in 0.5.2 |
Thanks! Also, rubocop may have disabled I do remember it yelling at me about the method missing one, but I cant remember if it was useful or not. |
IIRC the MissingSuper is a recent replacement for the other one |
That's my understanding, but if the new MissingSuper is disabled here, you might want to turn back on MethodMissingSuper manually? |
My understanding is |
Interesting. Unless I'm missing something, it seems completely mistaken from Edit: I opened sorbet/sorbet#3388 |
Will reopen and see if this is a sorbet-specific thing and they're open to changing it. If so and nobody else raises issues I'll turn it back on |
@searls For what it's worth, I ran into this and was surprised outside of Sorbet a few months back: https://www.reddit.com/r/ruby/comments/hw8t6a/how_to_please_standardrbrubocop/. It also references another user in Rubocops tracker with a similar issue: rubocop/rubocop#3760 (comment). Do you feel that |
@ttilberg I agree, the rule doesn't quit work for |
@ttilberg if you look at @marcandre's PR, the feedback received was to document why this hook should be an exception. Since you've written about this before and care a lot about it, could you provide a documentation update for inclusion in that PR so that it can be merged upstream? |
No worries, I'm on it
:-)
…On Fri, Nov 20, 2020, 08:31 Justin Searls ***@***.***> wrote:
@ttilberg <https://github.com/ttilberg> if you look at @marcandre
<https://github.com/marcandre>'s PR
<rubocop/rubocop#9072>, the feedback received
was to document why this hook should be an exception. Since you've written
about this before and care a lot about it, could you provide a
documentation update for inclusion in that PR so that it can be merged
upstream?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#195 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAIH2XV7UKR2TDB2JYO7LTSQZVTRANCNFSM4QLOMJEA>
.
|
@will We are looking at turning this on because it's good for the average Rubyist. Has Sorbet changed? Or would this still be an issue? |
@jmkoni do you know of a way that I can test turning it back on in a single project, to see what happens? Something in that yaml file maybe? |
@will yeah! You should be able to do something like this, but instead enable |
with a parallel: true
ignore:
- 'sorbet/**/*'
- '**/*':
- Lint/ConstantDefinitionInBlock # sorbet enums
Lint/MissingSuper:
Enabled: true standardrb 0.11.0 doesn't report any problems, however I would have expected these two classes # typed: strict
# frozen_string_literal: true
class Factory
extend T::Sig
extend T::Helpers
abstract!
# sorbet workaround, ref: https://github.com/sorbet/sorbet/issues/2374
alias_method :initialize_abstract, :initialize
sig { params(params: T::Hash[Symbol, T.untyped]).void }
def initialize(params = {})
initialize_abstract
end
end
class ClusterFactory < Factory
extend T::Sig
sig { params(params: T::Hash[Symbol, T.untyped]).void }
def initialize(params = {})
# no super
end
end to have generated an error with this on? Unless that alias trick is also solving that problem, but it seems like it wouldn't. |
Oh I see it was supposed to be in This still breaks for me in that case above. Also in another area I'm on purpose not calling super in subclasses, but this is not related to anything sorbet. Either way I'm okay with whatever you all decide as long as I can still opt out of this check with an ignore, which I think would work like I'm doing for |
TIL (well, re-learned) that we allowed rule-specific ignores in Standard. That being the case I think we should turn this back on and hope Sorbet users who need this will find this issue. |
and upvote / comment on the issue I opened on Sorbet... |
Hi, I just updated and noticed this linter was turned on in 0.5.0.
It has a ton of problems for me since I use
abstract!
from sorbet. It generates an initialize method in the abstract parent class to raise an exception if anyone accidentally tries to instantiate it. All of the child classes have their owninitialize
methods and are correctly not callingsuper
, but the linter thinks they should be calling it.I'm going to disable it for myself in the yaml file, but wanted to open the issue here in case others have the same problem.
The text was updated successfully, but these errors were encountered: