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

Diagnose precedence inversion in a warning wave #46239

Merged
merged 5 commits into from
Jul 31, 2020
Merged

Diagnose precedence inversion in a warning wave #46239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5c46c10
Diagnose precedence inversion in a warning wave
Jul 22, 2020
9ffb6b1
Merge branch 'master' of https://github.com/dotnet/roslyn into master…
Jul 24, 2020
6f36cc0
Enhance diagnostic text.
Jul 27, 2020
a8a2b80
Merge branch 'master' of https://github.com/dotnet/roslyn into master…
Jul 30, 2020
b112997
Show diagnostics from a test.
Jul 30, 2020
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
1 change: 1 addition & 0 deletions docs/compilers/CSharp/Warnversion Warning Waves.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,4 @@ The table below describes all of the warnings controlled by warning levels `5` o
|------------|---------|-------------|
| CS7023 | 5 | [A static type is used in an 'is' or 'as' expression](https://github.com/dotnet/roslyn/issues/30198) |
| CS8073 | 5 | [Expression always true (or false) when comparing a struct to null](https://github.com/dotnet/roslyn/issues/45744) |
| CS8848 | 5 | [Diagnose precedence error with query expression](https://github.com/dotnet/roslyn/issues/30231) |
6 changes: 6 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6352,6 +6352,12 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_NonPrivateAPIInRecord" xml:space="preserve">
<value>Record member '{0}' must be private.</value>
</data>
<data name="WRN_PrecedenceInversion" xml:space="preserve">
<value>Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</value>
Copy link
Member

@cston cston Jul 30, 2020 •
edited by gafter
Loading

Choose a reason for hiding this comment

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

Operator '{0}' cannot be used here due to precedence [](start = 11, length = 52)

This is a warning so it seems confusing that the message is "Operator cannot be used here due to precedence." We're allowing the operator so presumably it can be used. Should the message be more specific? Perhaps "Operator cannot be used here with the usual precedence." #ByDesign

Copy link
Member Author

Choose a reason for hiding this comment

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

How would "usual" add any clarity? We're accepting the operator because we're not paying any attention to precedence at all.

In reply to: 463301820 [](ancestors = 463301820)

</data>
<data name="WRN_PrecedenceInversion_Title" xml:space="preserve">
<value>Operator cannot be used here due to precedence.</value>
</data>
<data name="IDS_FeatureModuleInitializers" xml:space="preserve">
<value>module initializers</value>
</data>
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1836,7 +1836,7 @@ internal enum ErrorCode

WRN_SwitchExpressionNotExhaustiveWithWhen = 8846,
WRN_SwitchExpressionNotExhaustiveForNullWithWhen = 8847,

WRN_PrecedenceInversion = 8848,
ERR_ExpressionTreeContainsWithExpression = 8849,
ERR_BadRecordDeclaration = 8850,

Expand Down
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ internal static int GetWarningLevel(ErrorCode code)
{
case ErrorCode.WRN_NubExprIsConstBool2:
case ErrorCode.WRN_StaticInAsOrIs:
case ErrorCode.WRN_PrecedenceInversion:
// Warning level 5 is exclusively for warnings introduced in the compiler
// shipped with dotnet 5 (C# 9) and that can be reported for pre-existing code.
return 5;
Expand Down
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Generated/ErrorFacts.Generated.cs
View file
Open in desktop

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

40 changes: 18 additions & 22 deletions src/Compilers/CSharp/Portable/Parser/LanguageParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10037,24 +10037,22 @@ private ExpressionSyntax ParseExpressionContinued(ExpressionSyntax leftOperand,
// We'll "take" this operator, as precedence is tentatively OK.
var opToken = this.EatContextualToken(tk);

if (leftOperand.Kind == SyntaxKind.IsPatternExpression || IsStrict)
{
var leftPrecedence = GetPrecedence(leftOperand.Kind);
if (newPrecedence > leftPrecedence)
{
// Normally, a left operand with a looser precedence will consume all right operands that
// have a tighter precedence. For example, in the expression `a + b * c`, the `* c` part
// will be consumed as part of the right operand of the addition. However, there are a
// few circumstances in which a tighter precedence is not consumed: that occurs when the
// left hand operator does not have an expression as its right operand. This occurs for
// the is-type operator and the is-pattern operator. Source text such as
// `a is {} + b` should produce a syntax error, as parsing the `+` with an `is`
// expression as its left operand would be a precedence inversion. Similarly, it occurs
// with an anonymous method expression or a lambda expression with a block body. No
// further parsing will find a way to fix things up, so we accept the operator but issue
// an error.
opToken = this.AddError(opToken, ErrorCode.ERR_UnexpectedToken, opToken.Text);
}
var leftPrecedence = GetPrecedence(leftOperand.Kind);
if (newPrecedence > leftPrecedence)
{
// Normally, a left operand with a looser precedence will consume all right operands that
// have a tighter precedence. For example, in the expression `a + b * c`, the `* c` part
// will be consumed as part of the right operand of the addition. However, there are a
// few circumstances in which a tighter precedence is not consumed: that occurs when the
// left hand operator does not have an expression as its right operand. This occurs for
// the is-type operator and the is-pattern operator. Source text such as
// `a is {} + b` should produce a syntax error, as parsing the `+` with an `is`
// expression as its left operand would be a precedence inversion. Similarly, it occurs
// with an anonymous method expression or a lambda expression with a block body. No
// further parsing will find a way to fix things up, so we accept the operator but issue
// a diagnostic.
ErrorCode errorCode = leftOperand.Kind == SyntaxKind.IsPatternExpression ? ErrorCode.ERR_UnexpectedToken : ErrorCode.WRN_PrecedenceInversion;
Copy link
Member

@cston cston Jul 30, 2020 •
edited by gafter
Loading

Choose a reason for hiding this comment

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

ErrorCode.ERR_UnexpectedToken [](start = 95, length = 29)

Aren't we reporting an error in the non-pattern case now, where previously there was only an error if using "/strict"? #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it is a warning : ErrorCode.WRN_PrecedenceInversion

In reply to: 463304547 [](ancestors = 463304547)

opToken = this.AddError(opToken, errorCode, opToken.Text);
}

if (doubleOp)
Expand Down Expand Up @@ -12446,9 +12444,9 @@ private QueryExpressionSyntax ParseQueryExpression(Precedence precedence)
this.EnterQuery();
var fc = this.ParseFromClause();
fc = CheckFeatureAvailability(fc, MessageID.IDS_FeatureQueryExpression);
if (precedence > Precedence.Assignment && IsStrict)
if (precedence > Precedence.Assignment)
{
fc = this.AddError(fc, ErrorCode.ERR_InvalidExprTerm, SyntaxFacts.GetText(SyntaxKind.FromKeyword));
fc = this.AddError(fc, ErrorCode.WRN_PrecedenceInversion, SyntaxFacts.GetText(SyntaxKind.FromKeyword));
}

var body = this.ParseQueryBody();
Expand Down Expand Up @@ -12695,8 +12693,6 @@ private QueryContinuationSyntax ParseQueryContinuation()
return _syntaxFactory.QueryContinuation(@into, name, body);
}

private bool IsStrict => this.Options.Features.ContainsKey("strict");

[Obsolete("Use IsIncrementalAndFactoryContextMatches")]
private new bool IsIncremental
{
Expand Down
10 changes: 10 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2406,6 +2406,16 @@
<target state="translated">Parametr musí mít při ukončení hodnotu jinou než null</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion">
<source>Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</source>
<target state="new">Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion_Title">
<source>Operator cannot be used here due to precedence.</source>
<target state="new">Operator cannot be used here due to precedence.</target>
<note />
</trans-unit>
<trans-unit id="WRN_ShouldNotReturn">
<source>A method marked [DoesNotReturn] should not return.</source>
<target state="translated">Metoda označená jako [DoesNotReturn] by neměla vracet hodnotu</target>
Expand Down
10 changes: 10 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2406,6 +2406,16 @@
<target state="translated">Der Parameter muss beim Beenden einen Wert ungleich NULL aufweisen.</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion">
<source>Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</source>
<target state="new">Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion_Title">
<source>Operator cannot be used here due to precedence.</source>
<target state="new">Operator cannot be used here due to precedence.</target>
<note />
</trans-unit>
<trans-unit id="WRN_ShouldNotReturn">
<source>A method marked [DoesNotReturn] should not return.</source>
<target state="translated">Eine mit [DoesNotReturn] gekennzeichnete Methode darf nicht zurückgegeben werden.</target>
Expand Down
10 changes: 10 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2406,6 +2406,16 @@
<target state="translated">El parámetro debe tener un valor que no sea nulo al salir.</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion">
<source>Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</source>
<target state="new">Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion_Title">
<source>Operator cannot be used here due to precedence.</source>
<target state="new">Operator cannot be used here due to precedence.</target>
<note />
</trans-unit>
<trans-unit id="WRN_ShouldNotReturn">
<source>A method marked [DoesNotReturn] should not return.</source>
<target state="translated">Un método marcado como [DoesNotReturn] no debe devolver nada.</target>
Expand Down
10 changes: 10 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2406,6 +2406,16 @@
<target state="translated">Le paramètre doit avoir une valeur non null au moment de la sortie.</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion">
<source>Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</source>
<target state="new">Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion_Title">
<source>Operator cannot be used here due to precedence.</source>
<target state="new">Operator cannot be used here due to precedence.</target>
<note />
</trans-unit>
<trans-unit id="WRN_ShouldNotReturn">
<source>A method marked [DoesNotReturn] should not return.</source>
<target state="translated">Une méthode marquée [DoesNotReturn] ne doit pas être retournée.</target>
Expand Down
10 changes: 10 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2406,6 +2406,16 @@
<target state="translated">Il parametro deve avere un valore non Null quando viene terminato.</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion">
<source>Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</source>
<target state="new">Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion_Title">
<source>Operator cannot be used here due to precedence.</source>
<target state="new">Operator cannot be used here due to precedence.</target>
<note />
</trans-unit>
<trans-unit id="WRN_ShouldNotReturn">
<source>A method marked [DoesNotReturn] should not return.</source>
<target state="translated">Un metodo contrassegnato con [DoesNotReturn] non deve essere terminare normalmente.</target>
Expand Down
10 changes: 10 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2406,6 +2406,16 @@
<target state="translated">終了時にパラメーターには null 以外の値が含まれている必要があります。</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion">
<source>Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</source>
<target state="new">Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion_Title">
<source>Operator cannot be used here due to precedence.</source>
<target state="new">Operator cannot be used here due to precedence.</target>
<note />
</trans-unit>
<trans-unit id="WRN_ShouldNotReturn">
<source>A method marked [DoesNotReturn] should not return.</source>
<target state="translated">[DoesNotReturn] とマークされたメソッドを返すことはできません。</target>
Expand Down
10 changes: 10 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2406,6 +2406,16 @@
<target state="translated">종료할 때 매개 변수는 null이 아닌 값을 가져야 합니다.</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion">
<source>Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</source>
<target state="new">Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion_Title">
<source>Operator cannot be used here due to precedence.</source>
<target state="new">Operator cannot be used here due to precedence.</target>
<note />
</trans-unit>
<trans-unit id="WRN_ShouldNotReturn">
<source>A method marked [DoesNotReturn] should not return.</source>
<target state="translated">[DoesNotReturn]으로 표시된 메서드는 반환하지 않아야 합니다.</target>
Expand Down
10 changes: 10 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2406,6 +2406,16 @@
<target state="translated">Parametr musi mieć wartość inną niż null podczas kończenia działania.</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion">
<source>Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</source>
<target state="new">Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion_Title">
<source>Operator cannot be used here due to precedence.</source>
<target state="new">Operator cannot be used here due to precedence.</target>
<note />
</trans-unit>
<trans-unit id="WRN_ShouldNotReturn">
<source>A method marked [DoesNotReturn] should not return.</source>
<target state="translated">Metoda z oznaczeniem [DoesNotReturn] nie powinna zwracać wartości.</target>
Expand Down
10 changes: 10 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2405,6 +2405,16 @@
<target state="translated">O parâmetro deve ter um valor não nulo ao sair.</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion">
<source>Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</source>
<target state="new">Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion_Title">
<source>Operator cannot be used here due to precedence.</source>
<target state="new">Operator cannot be used here due to precedence.</target>
<note />
</trans-unit>
<trans-unit id="WRN_ShouldNotReturn">
<source>A method marked [DoesNotReturn] should not return.</source>
<target state="translated">Um método marcado como [DoesNotReturn] não deve ser retornado.</target>
Expand Down
10 changes: 10 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2406,6 +2406,16 @@
<target state="translated">Параметр должен иметь значение, отличное от NULL, при выходе.</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion">
<source>Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</source>
<target state="new">Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion_Title">
<source>Operator cannot be used here due to precedence.</source>
<target state="new">Operator cannot be used here due to precedence.</target>
<note />
</trans-unit>
<trans-unit id="WRN_ShouldNotReturn">
<source>A method marked [DoesNotReturn] should not return.</source>
<target state="translated">Метод, помеченный [DoesNotReturn], не должен возвращать значение.</target>
Expand Down
10 changes: 10 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2406,6 +2406,16 @@
<target state="translated">Parametre çıkış yaparken null olmayan bir değere sahip olmalıdır.</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion">
<source>Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</source>
<target state="new">Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion_Title">
<source>Operator cannot be used here due to precedence.</source>
<target state="new">Operator cannot be used here due to precedence.</target>
<note />
</trans-unit>
<trans-unit id="WRN_ShouldNotReturn">
<source>A method marked [DoesNotReturn] should not return.</source>
<target state="translated">[DoesNotReturn] olarak işaretlenen bir metot, değer döndürmemelidir.</target>
Expand Down
10 changes: 10 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2406,6 +2406,16 @@
<target state="translated">退出时，参数必须具有非 null 值。</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion">
<source>Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</source>
<target state="new">Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion_Title">
<source>Operator cannot be used here due to precedence.</source>
<target state="new">Operator cannot be used here due to precedence.</target>
<note />
</trans-unit>
<trans-unit id="WRN_ShouldNotReturn">
<source>A method marked [DoesNotReturn] should not return.</source>
<target state="translated">不应返回标记为 [DoesNotReturn] 的方法。</target>
Expand Down
10 changes: 10 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2406,6 +2406,16 @@
<target state="translated">參數在結束時必須具有非 Null 值。</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion">
<source>Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</source>
<target state="new">Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</target>
<note />
</trans-unit>
<trans-unit id="WRN_PrecedenceInversion_Title">
<source>Operator cannot be used here due to precedence.</source>
<target state="new">Operator cannot be used here due to precedence.</target>
<note />
</trans-unit>
<trans-unit id="WRN_ShouldNotReturn">
<source>A method marked [DoesNotReturn] should not return.</source>
<target state="translated">標記 [DoesNotReturn] 的方法不應傳回。</target>
Expand Down
Loading