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

Detect unused record parameters #48895

Merged
merged 7 commits into from
Oct 31, 2020
Merged

Detect unused record parameters #48895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0f631f6
Detect unused record parameters
jcouv Oct 23, 2020
acbc503
Tweak message
jcouv Oct 26, 2020
09dd1bc
Missing files
jcouv Oct 26, 2020
0170500
Merge remote-tracking branch 'dotnet/master' into unused-parameter
jcouv Oct 26, 2020
40d75ab
Add test case
jcouv Oct 26, 2020
f2a1ae8
Renamed to Unread. Address feedback from Aleksey
jcouv Oct 27, 2020
68d978f
Also count reads in lambdas
jcouv Oct 29, 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
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 @@ -6579,4 +6579,10 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_FunctionPointersCannotBeCalledWithNamedArguments" xml:space="preserve">
<value>A function pointer cannot be called with named arguments.</value>
</data>
<data name="WRN_UnreadRecordParameter" xml:space="preserve">
<value>Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</value>
Copy link
Contributor

@AlekseyTs AlekseyTs Oct 27, 2020 •
edited by jcouv
Loading

Choose a reason for hiding this comment

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

Did you forget to use it to initialize the property with that name? [](start = 38, length = 67)

I am not sure the second sentence is necessary. #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.

It's not necessary, but I think it's useful to guide users to the most likely solution. It helps for the primary scenario motivating this change.

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

</data>
<data name="WRN_UnreadRecordParameter_Title" xml:space="preserve">
<value>Parameter is unread. Did you forget to use it to initialize the property with that name?</value>
</data>
</root>
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1924,6 +1924,8 @@ internal enum ErrorCode

ERR_EqualityContractRequiresGetter = 8906,

WRN_UnreadRecordParameter = 8907,

#endregion diagnostics introduced for C# 9.0

// Note: you will need to re-generate compiler code after adding warnings (eng\generate-compiler-code.cmd)
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 @@ -470,6 +470,7 @@ internal static int GetWarningLevel(ErrorCode code)
case ErrorCode.WRN_ParameterNotNullIfNotNull:
case ErrorCode.WRN_ReturnNotNullIfNotNull:
case ErrorCode.WRN_AnalyzerReferencesFramework:
case ErrorCode.WRN_UnreadRecordParameter:
return 1;
default:
return 0;
Expand Down
33 changes: 33 additions & 0 deletions src/Compilers/CSharp/Portable/FlowAnalysis/DefiniteAssignment.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@ internal partial class DefiniteAssignmentPass : LocalDataFlowPass<
/// </summary>
private readonly PooledHashSet<LocalSymbol> _usedVariables = PooledHashSet<LocalSymbol>.GetInstance();

#nullable enable
/// <summary>
/// Parameters of record primary constructors that were read anywhere.
/// </summary>
private PooledHashSet<ParameterSymbol>? _readParameters;
#nullable disable

/// <summary>
/// Variables that were used anywhere, in the sense required to suppress warnings about
/// unused variables.
Expand Down Expand Up @@ -186,6 +193,7 @@ internal DefiniteAssignmentPass(
protected override void Free()
{
_usedVariables.Free();
_readParameters?.Free();
_usedLocalFunctions.Free();
_writtenVariables.Free();
_capturedVariables.Free();
Expand Down Expand Up @@ -506,6 +514,17 @@ protected void Analyze(ref bool badRegion, DiagnosticBag diagnostics)
}

diagnostics.AddRange(this.Diagnostics);

if (CurrentSymbol is SynthesizedRecordConstructor)
{
foreach (ParameterSymbol parameter in MethodParameters)
{
if (_readParameters?.Contains(parameter) != true)
{
diagnostics.Add(ErrorCode.WRN_UnreadRecordParameter, parameter.Locations.FirstOrNone(), parameter.Name);
}
}
}
}
}

Expand Down Expand Up @@ -550,6 +569,14 @@ private void NoteCaptured(Symbol variable)

#region Tracking reads/writes of variables for warnings

private void NoteRecordParameterReadIfNeeded(Symbol symbol)
{
if (symbol is ParameterSymbol { ContainingSymbol: SynthesizedRecordConstructor } parameter)
{
_readParameters ??= PooledHashSet<ParameterSymbol>.GetInstance();
_readParameters.Add(parameter);
}
}
protected virtual void NoteRead(
Symbol variable,
ParameterSymbol rangeVariableUnderlyingParameter = null)
Expand All @@ -560,6 +587,8 @@ protected virtual void NoteRead(
_usedVariables.Add(local);
}

NoteRecordParameterReadIfNeeded(variable);

var localFunction = variable as LocalFunctionSymbol;
if ((object)localFunction != null)
{
Expand Down Expand Up @@ -1878,6 +1907,10 @@ public override BoundNode VisitParameter(BoundParameter node)
{
CheckAssigned(node.ParameterSymbol, node.Syntax);
}
else
{
NoteRecordParameterReadIfNeeded(node.ParameterSymbol);
}

return null;
}
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.

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 @@ -2881,6 +2881,16 @@
<target state="needs-review-translation">Není inicializované pole, které nemůže mít hodnotu null. Zvažte možnost deklarovat ho jako typ s možnou hodnotou null.</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter">
<source>Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter_Title">
<source>Parameter is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UseDefViolation">
<source>Use of unassigned local variable '{0}'</source>
<target state="new">Use of unassigned local variable '{0}'</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 @@ -2881,6 +2881,16 @@
<target state="needs-review-translation">Das Non-Nullable-Feld ist nicht initialisiert. Deklarieren Sie das Feld ggf. als "Nullable".</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter">
<source>Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter_Title">
<source>Parameter is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UseDefViolation">
<source>Use of unassigned local variable '{0}'</source>
<target state="new">Use of unassigned local variable '{0}'</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 @@ -2881,6 +2881,16 @@
<target state="needs-review-translation">El campo que acepta valores NULL está sin inicializar. Considere la posibilidad de declararlo como que acepta valores NULL.</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter">
<source>Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter_Title">
<source>Parameter is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UseDefViolation">
<source>Use of unassigned local variable '{0}'</source>
<target state="new">Use of unassigned local variable '{0}'</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 @@ -2881,6 +2881,16 @@
<target state="needs-review-translation">Le champ non-nullable n'est pas initialisé. Déclarez-le comme étant nullable.</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter">
<source>Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter_Title">
<source>Parameter is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UseDefViolation">
<source>Use of unassigned local variable '{0}'</source>
<target state="new">Use of unassigned local variable '{0}'</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 @@ -2881,6 +2881,16 @@
<target state="needs-review-translation">Il campo che non ammette i valori Null non è inizializzato. Provare a dichiararlo in modo che ammetta i valori Null.</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter">
<source>Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter_Title">
<source>Parameter is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UseDefViolation">
<source>Use of unassigned local variable '{0}'</source>
<target state="new">Use of unassigned local variable '{0}'</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 @@ -2881,6 +2881,16 @@
<target state="needs-review-translation">Null 非許容フィールドは初期化されていません。null 許容として宣言することを検討してください。</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter">
<source>Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter_Title">
<source>Parameter is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UseDefViolation">
<source>Use of unassigned local variable '{0}'</source>
<target state="new">Use of unassigned local variable '{0}'</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 @@ -2881,6 +2881,16 @@
<target state="needs-review-translation">null을 허용하지 않는 필드가 초기화되지 않았습니다. nullable로 선언하는 것이 좋습니다.</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter">
<source>Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter_Title">
<source>Parameter is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UseDefViolation">
<source>Use of unassigned local variable '{0}'</source>
<target state="new">Use of unassigned local variable '{0}'</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 @@ -2881,6 +2881,16 @@
<target state="needs-review-translation">Nienullowalne pole jest niezainicjowane. Rozważ zadeklarowanie go jako nullowalnego.</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter">
<source>Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter_Title">
<source>Parameter is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UseDefViolation">
<source>Use of unassigned local variable '{0}'</source>
<target state="new">Use of unassigned local variable '{0}'</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 @@ -2879,6 +2879,16 @@
<target state="needs-review-translation">O campo não anulável não foi inicializado. Considere declará-lo como anulável.</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter">
<source>Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter_Title">
<source>Parameter is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UseDefViolation">
<source>Use of unassigned local variable '{0}'</source>
<target state="new">Use of unassigned local variable '{0}'</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 @@ -2881,6 +2881,16 @@
<target state="needs-review-translation">Поле, не допускающее значение NULL, не инициализировано. Рекомендуется объявить его как допускающее значение NULL.</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter">
<source>Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter_Title">
<source>Parameter is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UseDefViolation">
<source>Use of unassigned local variable '{0}'</source>
<target state="new">Use of unassigned local variable '{0}'</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 @@ -2881,6 +2881,16 @@
<target state="needs-review-translation">Null değer atanamayan alan başlatılmadı. Bunu null değer atanabilir olarak tanımlamayı deneyin.</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter">
<source>Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter_Title">
<source>Parameter is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UseDefViolation">
<source>Use of unassigned local variable '{0}'</source>
<target state="new">Use of unassigned local variable '{0}'</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 @@ -2881,6 +2881,16 @@
<target state="needs-review-translation">不可为 null 的字段未初始化。请考虑声明为可以为 null。</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter">
<source>Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter_Title">
<source>Parameter is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UseDefViolation">
<source>Use of unassigned local variable '{0}'</source>
<target state="new">Use of unassigned local variable '{0}'</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 @@ -2881,6 +2881,16 @@
<target state="needs-review-translation">不可為 null 的欄位未初始化。請考慮宣告為可為 null。</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter">
<source>Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter '{0}' is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UnreadRecordParameter_Title">
<source>Parameter is unread. Did you forget to use it to initialize the property with that name?</source>
<target state="new">Parameter is unread. Did you forget to use it to initialize the property with that name?</target>
<note />
</trans-unit>
<trans-unit id="WRN_UseDefViolation">
<source>Use of unassigned local variable '{0}'</source>
<target state="new">Use of unassigned local variable '{0}'</target>
Expand Down
7 changes: 5 additions & 2 deletions src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68086,7 +68086,7 @@ record Rec2<T>(T t = default)
T t { get; } = t; // 2
}

record Rec3<T>(T t = default)
record Rec3<T>(T t = default) // 3
{
T t { get; } = default!;
}
Expand All @@ -68098,7 +68098,10 @@ record Rec3<T>(T t = default)
Diagnostic(ErrorCode.WRN_NullReferenceAssignment, "T t = default").WithLocation(2, 16),
// (8,19): warning CS8601: Possible null reference assignment.
// T t { get; } = t; // 2
Diagnostic(ErrorCode.WRN_NullReferenceAssignment, "t").WithLocation(8, 19)
Diagnostic(ErrorCode.WRN_NullReferenceAssignment, "t").WithLocation(8, 19),
// (11,18): warning CS8907: Parameter 't' is unread. Did you forget to use it to initialize the property with that name?
// record Rec3<T>(T t = default) // 3
Diagnostic(ErrorCode.WRN_UnreadRecordParameter, "t").WithArguments("t").WithLocation(11, 18)
);
}

Expand Down
Loading