-
Notifications
You must be signed in to change notification settings - Fork 89
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
Record method definitions via accessors #1037
base: master
Are you sure you want to change the base?
Record method definitions via accessors #1037
Conversation
@@ -3347,6 +3347,23 @@ def type_send(node, send_node:, block_params:, block_body:, unwrap: false, tapp: | |||
constr = self | |||
else | |||
receiver, method_name, *arguments = send_node.children | |||
|
|||
case method_name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this is the right place to do this.
|
||
attr_reader bar: untyped | ||
attr_writer baz: untyped | ||
attr_accessor qux: untyped |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without the fix, reproduces the issue. With the fix it passes by not having errors in test_expectations.yml
.
Should this be anywhere else instead?
Got this in CI:
Not too sure how it's related. I'll try to take a look. |
So actually
The test failure is there:
With an opaque:
Full output on `master`
Full output on this branch
|
efa0afe
to
a64531f
Compare
The test hinges on a plain `attr_reader` going from being annotated to unannotated. Before the change it went from a non-error state to an error state, which is used as a criteria for the LSP to After the change the annotation is not required anymore, therefore the LSP test endlessly waits for an error that never occurs. By moving to another type of error the LSP test now passes again. In addition the annotation was removed as it is unneeded now.
@soutaro tests are all passing now. I feel like littering code with Does the proposed code here look OK to you? |
Attempts to fix #1036