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.

Sign up for GitHub

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

Jump to bottom

Fix to #20612 - Throw helpful translation exception for String.Equals(String, StringComparison) #20663

Merged
merged 1 commit into from
May 13, 2020
Merged

Fix to #20612 - Throw helpful translation exception for String.Equals(String, StringComparison) #20663

merged 1 commit into from
May 13, 2020

Conversation

maumar
Copy link
Contributor

@maumar maumar commented Apr 16, 2020

Adding state to ExpressionTranslatingExpressionVisitor that stores the information about why translation failed. The information is bubbled up and added to the "translation failed" exception.
Currently only doing this for string.Equals and non-mapped member properties

@maumar maumar requested review from roji and smitpatel April 16, 2020 22:09
@maumar maumar force-pushed the better_exceptions branch from d60c77c to a9c4577 Compare April 16, 2020 22:13
@maumar maumar force-pushed the better_exceptions branch from a9c4577 to 75d4d6d Compare April 20, 2020 04:04
@roji roji mentioned this pull request Apr 20, 2020
Closed
smitpatel
smitpatel reviewed Apr 27, 2020
View reviewed changes
Copy link
Contributor

@smitpatel smitpatel left a comment

Choose a reason for hiding this comment

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

Add a test where both the errors are happening in same expression so we see results of concatenation.
Also paste output of each error and combination in PR so we can see what exactly user will see.

@maumar maumar force-pushed the better_exceptions branch from 75d4d6d to e7e740d Compare April 27, 2020 22:08
@maumar
Copy link
Contributor Author

maumar commented Apr 27, 2020

@smitpatel new version up

@maumar maumar force-pushed the better_exceptions branch 2 times, most recently from 93acd74 to 326b684 Compare April 27, 2020 23:57
@maumar
Copy link
Contributor Author

maumar commented May 7, 2020

ping

@smitpatel
Copy link
Contributor

Rebase?

@maumar maumar force-pushed the better_exceptions branch from 326b684 to d349425 Compare May 7, 2020 20:41
@maumar
Copy link
Contributor Author

maumar commented May 7, 2020

@smitpatel done

@JeremyLikness
Copy link
Member

@maumar here is a fwlink instead of aka.ms format (thanks @smitpatel) https://go.microsoft.com/fwlink/?linkid=2129535

@maumar maumar force-pushed the better_exceptions branch from d349425 to e228277 Compare May 11, 2020 02:22
@maumar
Copy link
Contributor Author

maumar commented May 12, 2020

ready again @smitpatel

@smitpatel
Copy link
Contributor

Also add member not mapped for indexer properties.

@maumar maumar force-pushed the better_exceptions branch from e228277 to 60ccd6e Compare May 12, 2020 18:02
@maumar maumar force-pushed the better_exceptions branch from 60ccd6e to 5ca6ae4 Compare May 12, 2020 21:07
@maumar
Fix to #20612 - Throw helpful translation exception for String.Equals…
9e0f05b 
…(String, StringComparison)

Adding state to ExpressionTranslatingExpressionVisitor that stores the information about why translation failed. The information is bubbled up and added to the "translation failed" exception.
Currently only doing this for string.Equals and non-mapped member properties
@maumar maumar force-pushed the better_exceptions branch from 5ca6ae4 to 9e0f05b Compare May 13, 2020 04:13
@maumar maumar merged commit 9e0f05b into master May 13, 2020
@maumar maumar deleted the better_exceptions branch May 13, 2020 05:11
@maumar maumar linked an issue May 13, 2020 that may be closed by this pull request
Throw helpful translation exception for String.Equals(String, StringComparison) #20612
Closed
@ajcvickers ajcvickers mentioned this pull request May 14, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ajcvickers ajcvickers ajcvickers left review comments

@roji roji roji left review comments

@JeremyLikness JeremyLikness JeremyLikness left review comments

@smitpatel smitpatel smitpatel approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Throw helpful translation exception for String.Equals(String, StringComparison)
5 participants
@maumar @smitpatel @JeremyLikness @ajcvickers @roji