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

improved error message in raise_not_initialized_in_all_initialize #3392

Closed
wants to merge 1 commit into from
Closed

improved error message in raise_not_initialized_in_all_initialize #3392

wants to merge 1 commit into from

Conversation

masukomi
Copy link
Contributor

@masukomi masukomi commented Oct 7, 2016

Disucssion in #3387 raised idea that this error message could be improved. Here's an attempt to do that.

Please scroll right on the diff. I've added "Indirect initialization is not supported." at the end but github isn't making that obvious.

Question: would it be better to say "Indirect initialization is not currently supported." ? I'm not sure if there are plans to change that.

cc @luislavena @kirbyfan64 @ozra (commenters on original issue)

end

private def raise_not_initialized_in_all_initialize(location : Location, name, owner)
raise TypeException.new "instance variable '#{name}' of #{owner} was not initialized in all of the 'initialize' methods, rendering it nilable", location
raise TypeException.new "instance variable '#{name}' of #{owner} was not initialized directly in all of the 'initialize' methods, rendering it nilable. Indirect initialization is not supported.", location
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please run crystal tool format on the source code to ensure the format/spacing is correct.

@@ -630,11 +630,11 @@ struct Crystal::TypeDeclarationProcessor
end

private def raise_not_initialized_in_all_initialize(node : ASTNode, name, owner)
node.raise "instance variable '#{name}' of #{owner} was not initialized in all of the 'initialize' methods, rendering it nilable"
node.raise "instance variable '#{name}' of #{owner} was not initialized directly in all of the 'initialize' methods, rendering it nilable. Indirect initialization is not supported."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the error message can be explained in a more verbose way, similar to raise_doesnt_explicitly_initializes multi-line error message?

@luislavena
Copy link
Contributor

There are also failures due the change of error message and not updating the spec, if you can take a look to all that will be great.

@masukomi
Copy link
Contributor Author

masukomi commented Oct 7, 2016

will do all suggested things. might not get to it until this evening or tomorrow morning.

@straight-shoota
Copy link
Member

@masukomi It has been a few evenings and mornings since october... ;)
Are you still up to address the comments?

@straight-shoota
Copy link
Member

This can be closed (superseeded by #4696)

@ysbaddaden ysbaddaden closed this Apr 5, 2018
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

Successfully merging this pull request may close these issues.

None yet

4 participants