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

Add failure text to the end of the metadata document if decompilation fails #24188

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,15 @@ public async Task<MetadataAsSourceFile> GetGeneratedFileAsync(Project project, I
}
catch (Exception e) when (FatalError.ReportWithoutCrashUnlessCanceled(e))
{
// Insert the exception message at the top of the fallback Metadata As Source document to
// help with diagnosing failures. When the generated source is added to the document, this
// comment will end up at the bottom.
var failureText = e.ToString();
var failureTextAsComment = "// " + failureText.Replace("\n", "\n// ") + Environment.NewLine;
Copy link
Member

Choose a reason for hiding this comment

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

Sorry one more thing that would be nice: add some text along the lines of "the decompilation failed, and here was the failure". Otherwise people might wonder why there's a random exception at the end of the file.


var sourceText = await temporaryDocument.GetTextAsync(cancellationToken).ConfigureAwait(false);
temporaryDocument = temporaryDocument.WithText(sourceText.Replace(new TextSpan(0, 0), failureTextAsComment));
Copy link
Member

Choose a reason for hiding this comment

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

The file is empty up to this point, right? So the new TextSpan(0,0) doesn't really matter what it is before? Add a comment?

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 might or might not be empty at this point. The comment already says it's placed at the top (which can be seen) and ends up at the bottom (covered by tests).

Copy link
Member

Choose a reason for hiding this comment

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

When is it not already empty?

Copy link
Member

Choose a reason for hiding this comment

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

"Insert this at the top" implies there's something after our insertion point, but the test doesn't show anything after. Either we can add an assert here that the existing file is empty, or we're missing a test case.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not aware of a case where it's not empty here. However, the code added by this pull request adheres to the stated behavior whether or not the document is empty at this point.


useDecompiler = false;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@

using System;
using System.Collections.Generic;
using System.Globalization;
using System.IO;
using System.Linq;
using System.Security;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces;
using Microsoft.CodeAnalysis.Text;
using Microsoft.VisualStudio.Composition;
using Microsoft.VisualStudio.Text;
using Roslyn.Test.Utilities;
using Roslyn.Utilities;
Expand All @@ -33,7 +35,8 @@ public static TestContext Create(
bool includeXmlDocComments = false,
string sourceWithSymbolReference = null,
string languageVersion = null,
string metadataLanguageVersion = null)
string metadataLanguageVersion = null,
ExportProvider exportProvider = null)
{
projectLanguage ??= LanguageNames.CSharp;
metadataSources ??= SpecializedCollections.EmptyEnumerable<string>();
Expand All @@ -43,7 +46,8 @@ public static TestContext Create(

var workspace = CreateWorkspace(
projectLanguage, metadataSources, includeXmlDocComments,
sourceWithSymbolReference, languageVersion, metadataLanguageVersion);
sourceWithSymbolReference, languageVersion, metadataLanguageVersion,
exportProvider);
return new TestContext(workspace);
}

Expand Down Expand Up @@ -95,22 +99,23 @@ private static string GetSpaceSeparatedTokens(string source)
return string.Join(" ", tokens);
}

public void VerifyResult(MetadataAsSourceFile file, string expected)
public void VerifyResult(MetadataAsSourceFile file, Lazy<string> expected)
{
var actual = File.ReadAllText(file.FilePath).Trim();
var actualSpan = file.IdentifierLocation.SourceSpan;
var expectedText = expected.Value;

// Compare exact texts and verify that the location returned is exactly that
// indicated by expected
MarkupTestFile.GetSpan(expected, out expected, out var expectedSpan);
AssertEx.EqualOrDiff(expected, actual);
MarkupTestFile.GetSpan(expectedText, out expectedText, out var expectedSpan);
AssertEx.EqualOrDiff(expectedText, actual);
Assert.Equal(expectedSpan.Start, actualSpan.Start);
Assert.Equal(expectedSpan.End, actualSpan.End);
}

public async Task GenerateAndVerifySourceAsync(string symbolMetadataName, string expected, Project project = null)
public async Task GenerateAndVerifySourceAsync(string symbolMetadataName, Lazy<string> expected, Project project = null, bool allowDecompilation = false)
{
var result = await GenerateSourceAsync(symbolMetadataName, project);
var result = await GenerateSourceAsync(symbolMetadataName, project, allowDecompilation);
VerifyResult(result, expected);
}

Expand Down Expand Up @@ -215,7 +220,8 @@ private static string DeduceLanguageString(string input)
private static TestWorkspace CreateWorkspace(
string projectLanguage, IEnumerable<string> metadataSources,
bool includeXmlDocComments, string sourceWithSymbolReference,
string languageVersion, string metadataLanguageVersion)
string languageVersion, string metadataLanguageVersion,
ExportProvider exportProvider)
{
var languageVersionAttribute = languageVersion is null ? "" : $@" LanguageVersion=""{languageVersion}""";

Expand Down Expand Up @@ -252,7 +258,7 @@ private static TestWorkspace CreateWorkspace(
</Project>
</Workspace>");

return TestWorkspace.Create(xmlString);
return TestWorkspace.Create(xmlString, exportProvider: exportProvider);
}

internal Document GetDocument(MetadataAsSourceFile file)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.Test.Utilities;
using Microsoft.VisualStudio.Composition;
using Roslyn.Utilities;
using Xunit;

Expand All @@ -14,10 +16,10 @@ namespace Microsoft.CodeAnalysis.Editor.UnitTests.MetadataAsSource
public abstract partial class AbstractMetadataAsSourceTests
{
internal static async Task GenerateAndVerifySourceAsync(
string metadataSource, string symbolName, string projectLanguage, string expected, bool includeXmlDocComments = false, string languageVersion = null, string metadataLanguageVersion = null)
string metadataSource, string symbolName, string projectLanguage, Lazy<string> expected, bool allowDecompilation = false, bool includeXmlDocComments = false, string languageVersion = null, string metadataLanguageVersion = null, ExportProvider exportProvider = null)
{
using var context = TestContext.Create(projectLanguage, SpecializedCollections.SingletonEnumerable(metadataSource), includeXmlDocComments, languageVersion: languageVersion, metadataLanguageVersion: metadataLanguageVersion);
await context.GenerateAndVerifySourceAsync(symbolName, expected);
using var context = TestContext.Create(projectLanguage, SpecializedCollections.SingletonEnumerable(metadataSource), includeXmlDocComments, languageVersion: languageVersion, metadataLanguageVersion: metadataLanguageVersion, exportProvider: exportProvider);
await context.GenerateAndVerifySourceAsync(symbolName, expected, allowDecompilation: allowDecompilation);
}

internal static async Task GenerateAndVerifySourceLineAsync(string source, string language, string expected)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.MetadataAsSource;
using Microsoft.CodeAnalysis.Test.Utilities;
Expand All @@ -21,15 +22,15 @@ Module M
Public Class D
End Class
End Module";
await GenerateAndVerifySourceAsync(metadataSource, "M+D", LanguageNames.VisualBasic, $@"#Region ""{FeaturesResources.Assembly} ReferencedAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null""
await GenerateAndVerifySourceAsync(metadataSource, "M+D", LanguageNames.VisualBasic, new Lazy<string>(() => $@"#Region ""{FeaturesResources.Assembly} ReferencedAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null""
' {CodeAnalysisResources.InMemoryAssembly}
#End Region
Friend Module M
Public Class [|D|]
Public Sub New()
End Class
End Module");
End Module"));
}

// This test depends on the version of mscorlib used by the TestWorkspace and may
Expand Down Expand Up @@ -65,7 +66,7 @@ End Class
End Namespace";

using var context = TestContext.Create(LanguageNames.VisualBasic);
await context.GenerateAndVerifySourceAsync("System.ObsoleteAttribute", expected);
await context.GenerateAndVerifySourceAsync("System.ObsoleteAttribute", new Lazy<string>(() => expected));
}

[Fact, Trait(Traits.Feature, Traits.Features.MetadataAsSource)]
Expand Down
Loading