Skip to content

Commit

Permalink
Fixup partial property API usage (#74951)
Browse files Browse the repository at this point in the history
  • Loading branch information
RikkiGibson authored Sep 5, 2024
1 parent 07d7a21 commit 2ff1a47
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -396,8 +396,6 @@ internal Variables GetRootScope()

internal Variables? GetVariablesForMethodScope(MethodSymbol method)
{
// https://github.com/dotnet/roslyn/issues/73772: is this needed if we also delete the weird cascading in EnterParameters?
method = method.PartialImplementationPart ?? method;
var variables = this;
while (true)
{
Expand Down
4 changes: 0 additions & 4 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2762,10 +2762,6 @@ private void EnterParameters()
}
}

// The partial definition part may include optional parameters whose default values we want to simulate assigning at the beginning of the method
// https://github.com/dotnet/roslyn/issues/73772: is this actually used/meaningful?
methodSymbol = methodSymbol.PartialDefinitionPart ?? methodSymbol;

var methodParameters = methodSymbol.Parameters;
var signatureParameters = (_useDelegateInvokeParameterTypes ? _delegateInvokeMethod! : methodSymbol).Parameters;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1591,6 +1591,15 @@ internal override void ForceComplete(SourceLocation? locationOpt, Predicate<Symb
}

DeclaringCompilation.SymbolDeclaredEvent(this);
if (this.IsPartialDefinition())
{
if (_getMethod is not null)
DeclaringCompilation.SymbolDeclaredEvent(_getMethod);

if (_setMethod is not null)
DeclaringCompilation.SymbolDeclaredEvent(_setMethod);
}

var completedOnThisThread = _state.NotePartComplete(CompletionPart.FinishPropertyParameters);
Debug.Assert(completedOnThisThread);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3231,7 +3231,7 @@ static partial void PartialMethod()
}

[Theory, CombinatorialData, WorkItem(32702, "https://github.com/dotnet/roslyn/issues/71149")]
public async Task TestPartialFileSymbolEndDiagnosticsAsync(bool separateFiles)
public async Task TestPartialMethodFileSymbolEndDiagnosticsAsync(bool separateFiles)
{
string definition1 = @"
internal partial class Test
Expand Down Expand Up @@ -3276,6 +3276,52 @@ internal partial class Test
);
}

[Theory, CombinatorialData, WorkItem(32702, "https://github.com/dotnet/roslyn/issues/71149")]
public async Task TestPartialPropertyFileSymbolEndDiagnosticsAsync(bool separateFiles)
{
string definition1 = @"
internal partial class Test
{
private partial object Prop { get; set; }
public Test(object _) { }
}";
string definition2 = @"
internal partial class Test
{
private partial object Prop { get => new(); set { } }
}";

string source1, source2;
if (separateFiles)
{
source1 = definition1;
source2 = definition2;
}
else
{
source1 = definition1 + definition2;
source2 = string.Empty;
}

var compilation = CreateCompilationWithMscorlib461([source1, source2]);
compilation.VerifyDiagnostics();

var tree1 = compilation.SyntaxTrees[0];
var semanticModel1 = compilation.GetSemanticModel(tree1);
var analyzers = ImmutableArray.Create<DiagnosticAnalyzer>(new SymbolStartAnalyzer(topLevelAction: false, SymbolKind.NamedType));
var compilationWithAnalyzers = compilation.WithAnalyzers(analyzers);

// Requesting diagnostics on a single tree should run the SymbolStart/End actions on all the partials across the compilation
// and the analysis result should contain the diagnostics reported at SymbolEnd action.
var analysisResult = await compilationWithAnalyzers.GetAnalysisResultAsync(semanticModel1, filterSpan: null, analyzers, CancellationToken.None);
Assert.Empty(analysisResult.SyntaxDiagnostics);
Assert.Empty(analysisResult.SemanticDiagnostics);
var compilationDiagnostics = analysisResult.CompilationDiagnostics[analyzers[0]];
compilationDiagnostics.Verify(
Diagnostic("SymbolStartRuleId").WithArguments("Test", "Analyzer1").WithLocation(1, 1)
);
}

[Fact, WorkItem(922802, "https://dev.azure.com/devdiv/DevDiv/_workitems/edit/922802")]
public async Task TestAnalysisScopeForGetAnalyzerSemanticDiagnosticsAsync()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,13 +286,15 @@ partial class Class
"",
"Class",
"DefOnlyPartialProp",
"get_DefOnlyPartialProp",
"get_ImplOnlyPartialProp",
"get_NonPartialProp1",
"get_PartialProp",
"ImplOnlyPartialProp",
"N1",
"NonPartialProp1",
"PartialProp",
"set_DefOnlyPartialProp",
"set_ImplOnlyPartialProp",
"set_NonPartialProp1",
"set_PartialProp"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,13 +204,10 @@ void processMembers(IEnumerable<ISymbol> members)
memberSet ??= new HashSet<ISymbol>();
memberSet.Add(member);

// Ensure that we include symbols for both parts of partial methods.
// https://github.com/dotnet/roslyn/issues/73772: also cascade to partial property implementation part
if (member is IMethodSymbol method &&
!(method.PartialImplementationPart is null))
{
memberSet.Add(method.PartialImplementationPart);
}
if (member is IMethodSymbol { PartialImplementationPart: { } methodImplementation })
memberSet.Add(methodImplementation);
else if (member is IPropertySymbol { PartialImplementationPart: { } propertyImplementation })
memberSet.Add(propertyImplementation);
}

if (member is INamedTypeSymbol typeMember)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1117,10 +1117,14 @@ static bool shouldIncludeSymbol(ISymbolInternal symbol, SyntaxTree tree, Cancell
return true;
}
}

// https://github.com/dotnet/roslyn/issues/73772: should we also check IPropertySymbol?
// there is no interface IPropertySymbolInternal
// where are tests for this?
else if (symbol is IPropertySymbolInternal propertySymbol)
{
if (propertySymbol.PartialDefinitionPart?.IsDefinedInSourceTree(tree, definedWithinSpan: null, cancellationToken) == true
|| propertySymbol.PartialImplementationPart?.IsDefinedInSourceTree(tree, definedWithinSpan: null, cancellationToken) == true)
{
return true;
}
}

return false;
}
Expand Down

0 comments on commit 2ff1a47

Please sign in to comment.