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

Clarify diagnostic when constant pattern has non-constant conversion to input type #64741

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
2011410
Clarify diagnostics
ArcadeMode Oct 14, 2022
b00555d
update localised resources with changed english text
ArcadeMode Oct 14, 2022
b1d900f
relocate statement to only add one errocode to diagnostics
ArcadeMode Oct 14, 2022
4aad412
change variable usage and supress null warning
ArcadeMode Oct 14, 2022
540293f
change variable usage and supress null warning
ArcadeMode Oct 14, 2022
6dc0cbf
resolve enum conflict
ArcadeMode Oct 14, 2022
b6ff4d6
re-add errorcode to nonbuildonly
ArcadeMode Oct 15, 2022
abc35ce
rewrite binder to handle literal nulls and use correct variables for …
ArcadeMode Oct 15, 2022
d06febf
clarify error message
ArcadeMode Oct 15, 2022
5e13b6c
show null literal conversion error when relevant
ArcadeMode Oct 15, 2022
c9d58cc
update unit tests about null literals to expect valuecantbenull error
ArcadeMode Oct 15, 2022
ccbec6a
add unit tests related to workitem #63476 and implemented changes
ArcadeMode Oct 15, 2022
887bed0
fix formatting
ArcadeMode Oct 15, 2022
d8d71aa
fix overlooked test cases
ArcadeMode Oct 15, 2022
554394f
update test to not relate to workitem but simply demonstrates built i…
ArcadeMode Oct 15, 2022
2500017
correct message in comment
ArcadeMode Oct 15, 2022
676e236
Merge branch 'dotnet:main' into clarify-diagnostic-non-constant-conve…
ArcadeMode Oct 26, 2022
9c3e2ff
ErrorCode.cs formatting
ArcadeMode Oct 26, 2022
84e660f
not null assert instead of ! suppression
ArcadeMode Oct 26, 2022
5bc527a
test for explicit conversions + use symboldistinguisher
ArcadeMode Oct 26, 2022
6391b38
generalize error message
ArcadeMode Oct 26, 2022
42e816e
add unit test with explicit cast
ArcadeMode Oct 26, 2022
da83f74
fix unit test
ArcadeMode Oct 26, 2022
767dd67
dont unnecessarily test for null - symboldistinguisher handles it jus…
ArcadeMode Oct 27, 2022
4d67f82
update null literal tests
ArcadeMode Oct 27, 2022
f66b852
Merge branch 'main' into clarify-diagnostic-non-constant-conversion-i…
ArcadeMode Nov 17, 2022
7082eb9
move errorcode and fix number
ArcadeMode Nov 17, 2022
6079224
Merge branch 'main' into clarify-diagnostic-non-constant-conversion-i…
ArcadeMode Mar 18, 2023
f899ed8
fix errorcode, message and tests
ArcadeMode Mar 18, 2023
e966df6
update tests
ArcadeMode Mar 18, 2023
64c9594
set ERR_ConstantValueOfTypeExpected except when the inputtype is erro…
ArcadeMode Mar 19, 2023
c7047bb
update tests
ArcadeMode Mar 19, 2023
de4251e
formatting
ArcadeMode Mar 19, 2023
8a996ab
formatting
ArcadeMode Mar 19, 2023
601b59a
Merge remote-tracking branch 'origin/main' into clarify-diagnostic-no…
ArcadeMode Mar 22, 2023
ab89a3a
restore removed line
ArcadeMode Mar 22, 2023
4698f28
update tests
ArcadeMode Mar 22, 2023
feef4af
check for dynamic, object, valuetype and typeparams
ArcadeMode Mar 25, 2023
9aa4323
update tests
ArcadeMode Mar 25, 2023
5fdf010
add extra tests for diagnostic
ArcadeMode Mar 25, 2023
d17db0c
move test to more appropriate location
ArcadeMode Mar 28, 2023
eb43863
fix diagnostics comments
ArcadeMode Mar 28, 2023
bc11149
undo file changes
ArcadeMode Mar 28, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,16 @@ private BoundExpression BindExpressionForPatternContinued(
{
if (constantValueOpt == null)
{
diagnostics.Add(ErrorCode.ERR_ConstantExpected, patternExpression.Location);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the suggestion from #64741 (comment) was to not introduce any new logic, and change this to just use the new error code you added, passing in the inputType.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed the check for user defined conversions. However a few tests where the input type was unknown you'd end up with messages like Constant value of type '?' expected. Which I dont think improves upon the existing behavior.

For this reason I've now implemented to conditionally return the original ERR_ConstantExpected when the inputType is an error symbol. If this is undesirable please let me know.

var strippedInputType = inputType.StrippedType();
if (strippedInputType.Kind is not SymbolKind.ErrorType and not SymbolKind.DynamicType and not SymbolKind.TypeParameter &&
strippedInputType.SpecialType is not SpecialType.System_Object and not SpecialType.System_ValueType)
{
diagnostics.Add(ErrorCode.ERR_ConstantValueOfTypeExpected, patternExpression.Location, strippedInputType);
}
else
{
diagnostics.Add(ErrorCode.ERR_ConstantExpected, patternExpression.Location);
}
hasErrors = true;
}
else if (inputType.IsPointerType())
Expand Down
3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -7505,4 +7505,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_BadCaseInSwitchArm" xml:space="preserve">
<value>A switch expression arm does not begin with a 'case' keyword.</value>
</data>
<data name="ERR_ConstantValueOfTypeExpected" xml:space="preserve">
<value>A constant value of type '{0}' is expected</value>
</data>
</root>
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2189,6 +2189,8 @@ internal enum ErrorCode
ERR_BadStaticAfterUnsafe = 9133,

ERR_BadCaseInSwitchArm = 9134,
ERR_ConstantValueOfTypeExpected = 9135,

#endregion

// Note: you will need to do the following after adding warnings:
Expand Down
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2309,6 +2309,7 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
case ErrorCode.ERR_BadNullableReferenceTypeInUsingAlias:
case ErrorCode.ERR_BadStaticAfterUnsafe:
case ErrorCode.ERR_BadCaseInSwitchArm:
case ErrorCode.ERR_ConstantValueOfTypeExpected:
return false;
default:
// NOTE: All error codes must be explicitly handled in this switch statement
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Test/Emit/CodeGen/SwitchTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2623,10 +2623,10 @@ static void Main()
}";
var compilation = base.CreateCSharpCompilation(text);
compilation.VerifyDiagnostics(
// (8,18): error CS0150: A constant value is expected
// (8,18): error CS9135: A constant value of type 'int' is expected
// case i:
Diagnostic(ErrorCode.ERR_ConstantExpected, "i").WithLocation(8, 18)
);
Diagnostic(ErrorCode.ERR_ConstantValueOfTypeExpected, "i").WithArguments("int").WithLocation(8, 18)
);
}

[Fact, WorkItem(7625, "https://github.com/dotnet/roslyn/issues/7625")]
Expand Down
Loading