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

Fix null ref in initialize-parameter. #18487

Merged
merged 1 commit into from
Apr 6, 2017

Conversation

CyrusNajmabadi
Copy link
Member

No description provided.

@CyrusNajmabadi
Copy link
Member Author

Tagging @dotnet/roslyn-ide @jmarolf

Copy link
Contributor

@jmarolf jmarolf left a comment

Choose a reason for hiding this comment

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

👍

@CyrusNajmabadi CyrusNajmabadi merged commit 32869a8 into dotnet:master Apr 6, 2017
@CyrusNajmabadi CyrusNajmabadi deleted the addParameterNullRef branch April 6, 2017 03:37
@jasonmalinowski
Copy link
Member

Shouldn't that be getting an EndOfFile token with the parent as the compilation syntax?

@CyrusNajmabadi
Copy link
Member Author

GetPreviousToken on an EOF token can return a .None token with a null parent.

I considered this when i originally wrote the code. but i thought FirstAncestorOrSelf was an extension method and would handle the 'null' case (like IsKind and many of the other node-extension methods do). It turned out this was an instance method. Hence boom.

@jasonmalinowski
Copy link
Member

All the more reason to hate extension methods that accept null receivers. 😉

@CyrusNajmabadi
Copy link
Member Author

i love them! :D

@CyrusNajmabadi
Copy link
Member Author

though, with ?. they're probably not necessary any more.

@jmarolf
Copy link
Contributor

jmarolf commented Apr 6, 2017

@mattwar probably still gets flashbacks from when he changed all those extension methods to instance methods and discovered that all if them just returned null if passed null

@jasonmalinowski
Copy link
Member

There's a part of me that would be happy to see a change where everything did ?. But the bug fallout and massive diff would be crazy...

@Pilchie
Copy link
Member

Pilchie commented Apr 6, 2017

GetPreviousToken on an EOF token can return a .None token with a null parent.

The only case for that is an empty file, right?

@mattwar
Copy link
Contributor

mattwar commented Apr 6, 2017

sure would be nice to have null checking compiler.

@CyrusNajmabadi
Copy link
Member Author

The only case for that is an empty file, right?

Or any file when you're on the first token.

@Pilchie
Copy link
Member

Pilchie commented Apr 6, 2017

Ah, got it. I was confused by the reference to EOF token before.

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

Successfully merging this pull request may close these issues.

6 participants