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

Add weak heuristics in our designer scanning code to improve experience during things like configuration switch. #74130

Merged
merged 1 commit into from
Jun 26, 2024
Merged

Add weak heuristics in our designer scanning code to improve experience during things like configuration switch. #74130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
56 changes: 51 additions & 5 deletions src/Features/Core/Portable/DesignerAttribute/DesignerAttributeDiscoveryService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.ComponentModel;
using System.Composition;
Expand All @@ -30,6 +31,17 @@ namespace Microsoft.CodeAnalysis.DesignerAttribute;
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
internal sealed partial class DesignerAttributeDiscoveryService() : IDesignerAttributeDiscoveryService
{
/// <summary>
/// Ugly, but sufficient hack. During times where we're missing global usings (which may not always be available
/// while the sdk is regenerating/restoring on things like a tfm switch), we hardcode in knowledge we have about
/// which namespaces the core designable types are in. That way we can still make a solid guess about what the base
/// type is, even if we can't resolve it at this moment.
/// </summary>
private static readonly ImmutableArray<string> s_wellKnownDesignerNamespaces = [
"System.Windows.Forms.Form",
"System.Windows.Forms.Design",
"System.ComponentModel"];

/// <summary>
/// Cache from the individual references a project has, to a boolean specifying if reference knows about the
/// System.ComponentModel.DesignerCategoryAttribute attribute.
Expand Down Expand Up @@ -244,21 +256,21 @@ private async Task ScanForDesignerCategoryUsageAsync(
}

hasDesignerCategoryType ??= await HasDesignerCategoryTypeAsync(project, cancellationToken).ConfigureAwait(false);
var data = await ComputeDesignerAttributeDataAsync(project, documentId, filePath, hasDesignerCategoryType.Value).ConfigureAwait(false);
var data = await ComputeDesignerAttributeDataAsync(project, documentId, filePath, hasDesignerCategoryType.Value, existingInfo.category).ConfigureAwait(false);
if (data.Category != existingInfo.category)
results.Add((data, projectVersion));
}

return results.ToImmutableAndClear();

async Task<DesignerAttributeData> ComputeDesignerAttributeDataAsync(
Project project, DocumentId documentId, string filePath, bool hasDesignerCategoryType)
Project project, DocumentId documentId, string filePath, bool hasDesignerCategoryType, string? existingCategory)
{
// We either haven't computed the designer info, or our data was out of date. We need
// So recompute here. Figure out what the current category is, and if that's different
// from what we previously stored.
var category = await ComputeDesignerAttributeCategoryAsync(
hasDesignerCategoryType, project, documentId, cancellationToken).ConfigureAwait(false);
hasDesignerCategoryType, project, documentId, existingCategory, cancellationToken).ConfigureAwait(false);

return new DesignerAttributeData
{
Expand All @@ -270,7 +282,7 @@ async Task<DesignerAttributeData> ComputeDesignerAttributeDataAsync(
}

public static async Task<string?> ComputeDesignerAttributeCategoryAsync(
bool hasDesignerCategoryType, Project project, DocumentId documentId, CancellationToken cancellationToken)
bool hasDesignerCategoryType, Project project, DocumentId documentId, string? existingCategory, CancellationToken cancellationToken)
{
// simple case. If there's no DesignerCategory type in this compilation, then there's definitely no
// designable types.
Expand All @@ -292,10 +304,17 @@ async Task<DesignerAttributeData> ComputeDesignerAttributeDataAsync(
var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false);
var firstClassType = (INamedTypeSymbol)semanticModel.GetRequiredDeclaredSymbol(firstClass, cancellationToken);

foreach (var type in firstClassType.GetBaseTypesAndThis())
foreach (var type in GetBaseTypesAndThis(semanticModel.Compilation, firstClassType))
{
cancellationToken.ThrowIfCancellationRequested();

// If we hit an error type while walking up, then preserve the existing category. We do want a temporary
// invalid base type to not cause us to lose the existing category, causing a designable type to revert to
// an undesignable one. The designer can still support scenarios like this as it is itself error tolerant,
// falling back to the prior category in a case like this.
if (type is IErrorTypeSymbol errorType)
return existingCategory;

// See if it has the designer attribute on it. Use symbol-equivalence instead of direct equality
// as the symbol we have
var attribute = type.GetAttributes().FirstOrDefault(d => IsDesignerAttribute(d.AttributeClass));
Expand All @@ -305,6 +324,33 @@ async Task<DesignerAttributeData> ComputeDesignerAttributeDataAsync(

return null;

static IEnumerable<ITypeSymbol> GetBaseTypesAndThis(Compilation compilation, INamedTypeSymbol firstType)
{
var current = firstType;
while (current != null)
{
yield return current;
current = current.BaseType;

if (current is IErrorTypeSymbol errorType)
current = TryMapToNonErrorType(compilation, errorType);
}
}

static INamedTypeSymbol? TryMapToNonErrorType(Compilation compilation, IErrorTypeSymbol errorType)
{
foreach (var wellKnownNamespace in s_wellKnownDesignerNamespaces)
{
var wellKnownType = compilation.GetTypeByMetadataName($"{wellKnownNamespace}.{errorType.Name}");
Copy link
Member

Choose a reason for hiding this comment

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

🤔When will this happen to user? Namespace misspelling, why the namespace in SemanticModel is wrong?

Copy link
Member Author

Choose a reason for hiding this comment

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

Basically, as a restore happens, we can lose and then get references again. Also, generators may not run again until we know about them again, so we lost global usings.

This keeps everything still looking good (and usable for the designer side) while we get back into a good state again.

if (wellKnownType != null)
return wellKnownType;
}

// Couldn't find a match. Just return the error type as is. Caller will handle this case and try to
// preserve the existing category.
return errorType;
}

static bool IsDesignerAttribute(INamedTypeSymbol? attributeClass)
=> attributeClass is
{
Expand Down
162 changes: 98 additions & 64 deletions src/VisualStudio/CSharp/Test/DesignerAttribute/DesignerAttributeServiceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,85 +11,119 @@
using Roslyn.Utilities;
using Xunit;

namespace Microsoft.VisualStudio.LanguageServices.CSharp.UnitTests.DesignerAttributes
namespace Microsoft.VisualStudio.LanguageServices.CSharp.UnitTests.DesignerAttributes;

[UseExportProvider]
public sealed class DesignerAttributeServiceTests
{
[UseExportProvider]
public class DesignerAttributeServiceTests
[Fact]
public async Task NoDesignerTest1()
{
[Fact]
public async Task NoDesignerTest1()
{
var code = @"class Test { }";

await TestAsync(code, category: null);
}

[Fact]
public async Task NoDesignerOnSecondClass()
{

await TestAsync(
@"class Test1 { }
await TestAsync(@"class Test { }", category: null);
}

[System.ComponentModel.DesignerCategory(""Form"")]
class Test2 { }", category: null);
}
[Fact]
public async Task NoDesignerOnSecondClass()
{
await TestAsync(
"""
class Test1 { }

[Fact]
public async Task NoDesignerOnStruct()
{
[System.ComponentModel.DesignerCategory("Form")]
class Test2 { }
""", category: null);
}

await TestAsync(
@"
[System.ComponentModel.DesignerCategory(""Form"")]
struct Test1 { }", category: null);
}
[Fact]
public async Task NoDesignerOnStruct()
{
await TestAsync(
"""

[Fact]
public async Task NoDesignerOnNestedClass()
{
[System.ComponentModel.DesignerCategory("Form")]
struct Test1 { }
""", category: null);
}

await TestAsync(
@"class Test1
{
[System.ComponentModel.DesignerCategory(""Form"")]
class Test2 { }
}", category: null);
}
[Fact]
public async Task NoDesignerOnNestedClass()
{
await TestAsync(
"""
class Test1
{
[System.ComponentModel.DesignerCategory("Form")]
class Test2 { }
}
""", category: null);
}

[Fact]
public async Task SimpleDesignerTest()
{
[Fact]
public async Task SimpleDesignerTest()
{
await TestAsync(
"""
[System.ComponentModel.DesignerCategory("Form")]
class Test { }
""", "Form");
}

await TestAsync(
@"[System.ComponentModel.DesignerCategory(""Form"")]
class Test { }", "Form");
}
[Fact]
public async Task SimpleDesignerTest2()
{
await TestAsync(
"""
using System.ComponentModel;

[Fact]
public async Task SimpleDesignerTest2()
{
[DesignerCategory("Form")]
class Test { }
""", "Form");
}

await TestAsync(
@"using System.ComponentModel;
[Theory]
[InlineData(null)]
[InlineData("Form")]
[InlineData("Form1")]
public async Task TestUnboundBase1(string? existingCategory)
{
await TestAsync(
"""
namespace System.Windows.Forms
{
[System.ComponentModel.DesignerCategory("Form")]
public class Form { }
}

// The base type won't bind. That's ok. We should fallback to looking it up in a particular namespace.
// This should always work and not be impacted by the existing category.
class Test : Form { }
""", "Form", existingCategory);
}

[DesignerCategory(""Form"")]
class Test { }", "Form");
}
[Theory]
[InlineData(null)]
[InlineData("Form")]
public async Task TestUnboundBaseUseOldValueIfNotFound(string? category)
{
await TestAsync(
"""
// The base type won't bind. Return existing category if we have one.
class Test : Form { }
""", category: category, existingCategory: category);
}

private static async Task TestAsync(string codeWithMarker, string? category)
{
using var workspace = TestWorkspace.CreateCSharp(codeWithMarker, openDocuments: false);
private static async Task TestAsync(string codeWithMarker, string? category, string? existingCategory = null)
{
using var workspace = TestWorkspace.CreateCSharp(codeWithMarker, openDocuments: false);

var hostDocument = workspace.Documents.First();
var documentId = hostDocument.Id;
var document = workspace.CurrentSolution.GetRequiredDocument(documentId);
var hostDocument = workspace.Documents.First();
var documentId = hostDocument.Id;
var document = workspace.CurrentSolution.GetRequiredDocument(documentId);

var compilation = await document.Project.GetRequiredCompilationAsync(CancellationToken.None);
var actual = await DesignerAttributeDiscoveryService.ComputeDesignerAttributeCategoryAsync(
compilation.DesignerCategoryAttributeType() != null, document.Project, document.Id, CancellationToken.None);
var compilation = await document.Project.GetRequiredCompilationAsync(CancellationToken.None);
var actual = await DesignerAttributeDiscoveryService.ComputeDesignerAttributeCategoryAsync(
compilation.DesignerCategoryAttributeType() != null, document.Project, document.Id, existingCategory, CancellationToken.None);

Assert.Equal(category, actual);
}
Assert.Equal(category, actual);
}
}
Loading