-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Do not use declared constant's value inside a nameof() expression #503
Conversation
I also changed all of the tabs in SourceLocalSymbol to spaces. |
@VladimirReshetnikov @VSadov Can you please review this? |
@Jenkins test this please |
To: @AlekseyTs @agocke @VSadov @VladimirReshetnikov @jaredpar Can a couple of you please take a look? |
@@ -5244,7 +5245,7 @@ private static void CombineExtensionMethodArguments(BoundExpression receiver, An | |||
|
|||
ConstantValue constantValueOpt = null; | |||
|
|||
if (fieldSymbol.IsConst) | |||
if (fieldSymbol.IsConst && this.EnclosingNameofArgument == null) |
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.
Meaning "there is no enclosing nameof expression"
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.
Consider adding this as a comment
Does this need a test in the semantic model? |
@agocke What would you like to see tested? Or do you mean the semantic model may need to be adjusted to accomodate the change? |
@gafter Just that the semantic model also successfully binds without diagnostics. |
@@ -1138,5 +1138,22 @@ public static void Main(string[] args) | |||
Assert.Equal(CandidateReason.MemberGroup, symbolInfo.CandidateReason); | |||
Assert.Equal(1, symbolInfo.CandidateSymbols.Length); | |||
} | |||
|
|||
[Fact, WorkItem(40, "github.com/dotnet/roslyn")] | |||
public void ConstInitializerUsingSelf() |
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.
Please add similar test for VB.
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.
Doing so under #543.
LGTM. We should have the same test for VB. |
Do not use declared constant's value inside a nameof() expression
Fixes #40