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

Allow xml docs to still be created when 'emit metadata only' is on. #57667

Merged
merged 17 commits into from
Jan 21, 2022
Merged
Show file tree
Hide file tree
Changes from 13 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
18 changes: 11 additions & 7 deletions src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3286,15 +3286,15 @@ private void GenerateModuleInitializer(PEModuleBuilder moduleBeingBuilt, Diagnos
}
}

internal override bool GenerateResourcesAndDocumentationComments(
internal override bool GenerateResources(
CommonPEModuleBuilder moduleBuilder,
Stream? xmlDocStream,
Stream? win32Resources,
bool useRawWin32Resources,
string? outputNameOverride,
DiagnosticBag diagnostics,
CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();

// Use a temporary bag so we don't have to refilter pre-existing diagnostics.
DiagnosticBag? resourceDiagnostics = DiagnosticBag.GetInstance();

Expand All @@ -3306,11 +3306,15 @@ internal override bool GenerateResourcesAndDocumentationComments(
AddedModulesResourceNames(resourceDiagnostics),
resourceDiagnostics);

if (!FilterAndAppendAndFreeDiagnostics(diagnostics, ref resourceDiagnostics, cancellationToken))
{
return false;
}
return FilterAndAppendAndFreeDiagnostics(diagnostics, ref resourceDiagnostics, cancellationToken);
}

internal override bool GenerateDocumentationComments(
Stream? xmlDocStream,
string? outputNameOverride,
DiagnosticBag diagnostics,
CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();

// Use a temporary bag so we don't have to refilter pre-existing diagnostics.
Expand Down
247 changes: 246 additions & 1 deletion src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using System.Reflection.Metadata;
using System.Reflection.Metadata.Ecma335;
using System.Reflection.PortableExecutable;
using System.Text;
using System.Threading;
using Microsoft.CodeAnalysis.CSharp.Emit;
using Microsoft.CodeAnalysis.CSharp.Symbols;
Expand Down Expand Up @@ -178,7 +179,6 @@ namespace N.;
Diagnostic(ErrorCode.WRN_UnassignedInternalField, "field").WithArguments("N.X.field", "null").WithLocation(4, 21));
}

// Check that EmitMetadataOnly works
[Fact]
public void EmitMetadataOnly()
{
Expand Down Expand Up @@ -246,6 +246,251 @@ public static void Main()
}
}

[Fact]
public void EmitMetadataOnly_XmlDocs_NoDocMode_Success()
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
{
CSharpCompilation comp = CreateCompilation(@"
namespace Goo.Bar
{
/// <summary>This should be emitted</summary>
public class Test1
{
public static void SayHello()
{
EmitMetadataOnly_XmlDocs_NoDocMode_Success.WriteLine(""hello"");
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
", assemblyName: "test", parseOptions: CSharpParseOptions.Default.WithDocumentationMode(DocumentationMode.None));

EmitResult emitResult;
byte[] mdOnlyImage;
byte[] xmlDocBytes;

using (var peStream = new MemoryStream())
using (var xmlStream = new MemoryStream())
{
emitResult = comp.Emit(peStream, xmlDocumentationStream: xmlStream, options: new EmitOptions(metadataOnly: true));
mdOnlyImage = peStream.ToArray();
xmlDocBytes = xmlStream.ToArray();
}

Assert.True(emitResult.Success);
emitResult.Diagnostics.Verify();
Assert.True(mdOnlyImage.Length > 0, "no metadata emitted");
Assert.Equal(
"<?xml version=\"1.0\"?>\r\n<doc>\r\n <assembly>\r\n <name>test</name>\r\n </assembly>\r\n <members>\r\n </members>\r\n</doc>\r\n",
Encoding.UTF8.GetString(xmlDocBytes));
}

[Fact]
public void EmitMetadataOnly_XmlDocs_NoDocMode_SyntaxWarning()
{
CSharpCompilation comp = CreateCompilation(@"
namespace Goo.Bar
{
/// <summary>This should still emit
public class Test1
{
public static void SayHello()
{
Console.WriteLine(""hello"");
}
}
}
", assemblyName: "test", parseOptions: CSharpParseOptions.Default.WithDocumentationMode(DocumentationMode.None));

EmitResult emitResult;
byte[] mdOnlyImage;
byte[] xmlDocBytes;

using (var peStream = new MemoryStream())
using (var xmlStream = new MemoryStream())
{
emitResult = comp.Emit(peStream, xmlDocumentationStream: xmlStream, options: new EmitOptions(metadataOnly: true));
mdOnlyImage = peStream.ToArray();
xmlDocBytes = xmlStream.ToArray();
}

Assert.True(emitResult.Success);
emitResult.Diagnostics.Verify();

Assert.True(mdOnlyImage.Length > 0, "no metadata emitted");
Assert.Equal(
"<?xml version=\"1.0\"?>\r\n<doc>\r\n <assembly>\r\n <name>test</name>\r\n </assembly>\r\n <members>\r\n </members>\r\n</doc>\r\n",
Encoding.UTF8.GetString(xmlDocBytes));
}

[Fact]
public void EmitMetadataOnly_XmlDocs_DiagnoseDocMode_SyntaxWarning()
{
CSharpCompilation comp = CreateCompilation(@"
namespace Goo.Bar
{
/// <summary>This should still emit
public class Test1
{
public static void SayHello()
{
Console.WriteLine(""hello"");
}
}
}
", assemblyName: "test", parseOptions: CSharpParseOptions.Default.WithDocumentationMode(DocumentationMode.Diagnose));

EmitResult emitResult;
byte[] mdOnlyImage;
byte[] xmlDocBytes;

using (var peStream = new MemoryStream())
using (var xmlStream = new MemoryStream())
{
emitResult = comp.Emit(peStream, xmlDocumentationStream: xmlStream, options: new EmitOptions(metadataOnly: true));
mdOnlyImage = peStream.ToArray();
xmlDocBytes = xmlStream.ToArray();
}

// This should not fail the emit (as it's a warning).
Assert.True(emitResult.Success);
emitResult.Diagnostics.Verify(
// (5,1): warning CS1570: XML comment has badly formed XML -- 'Expected an end tag for element 'summary'.'
// public class Test1
Diagnostic(ErrorCode.WRN_XMLParseError, "").WithArguments("summary").WithLocation(5, 1),
// (7,28): warning CS1591: Missing XML comment for publicly visible type or member 'Test1.SayHello()'
// public static void SayHello()
Diagnostic(ErrorCode.WRN_MissingXMLComment, "SayHello").WithArguments("Goo.Bar.Test1.SayHello()").WithLocation(7, 28));

Assert.True(mdOnlyImage.Length > 0, "no metadata emitted");
Assert.Equal(
"<?xml version=\"1.0\"?>\r\n<doc>\r\n <assembly>\r\n <name>test</name>\r\n </assembly>\r\n <members>\r\n <!-- Badly formed XML comment ignored for member \"T:Goo.Bar.Test1\" -->\r\n </members>\r\n</doc>\r\n",
Encoding.UTF8.GetString(xmlDocBytes));
}

[Fact]
public void EmitMetadataOnly_XmlDocs_DiagnoseDocMode_SemanticWarning()
{
CSharpCompilation comp = CreateCompilation(@"
namespace Goo.Bar
{
/// <summary><see cref=""T""/></summary>
public class Test1
{
public static void SayHello()
{
Console.WriteLine(""hello"");
}
}
}
", assemblyName: "test", parseOptions: CSharpParseOptions.Default.WithDocumentationMode(DocumentationMode.Diagnose));

EmitResult emitResult;
byte[] mdOnlyImage;
byte[] xmlDocBytes;

using (var peStream = new MemoryStream())
using (var xmlStream = new MemoryStream())
{
emitResult = comp.Emit(peStream, xmlDocumentationStream: xmlStream, options: new EmitOptions(metadataOnly: true));
mdOnlyImage = peStream.ToArray();
xmlDocBytes = xmlStream.ToArray();
}

// This should not fail the emit (as it's a warning).
Assert.True(emitResult.Success);
emitResult.Diagnostics.Verify(
// (4,29): warning CS1574: XML comment has cref attribute 'T' that could not be resolved
// /// <summary><see cref="T"/></summary>
Diagnostic(ErrorCode.WRN_BadXMLRef, "T").WithArguments("T").WithLocation(4, 29),
// (7,28): warning CS1591: Missing XML comment for publicly visible type or member 'Test1.SayHello()'
// public static void SayHello()
Diagnostic(ErrorCode.WRN_MissingXMLComment, "SayHello").WithArguments("Goo.Bar.Test1.SayHello()").WithLocation(7, 28));

Assert.True(mdOnlyImage.Length > 0, "no metadata emitted");
Assert.Equal(
"<?xml version=\"1.0\"?>\r\n<doc>\r\n <assembly>\r\n <name>test</name>\r\n </assembly>\r\n <members>\r\n <member name=\"T:Goo.Bar.Test1\">\r\n <summary><see cref=\"!:T\"/></summary>\r\n </member>\r\n </members>\r\n</doc>\r\n",
Encoding.UTF8.GetString(xmlDocBytes));
}

[Fact]
public void EmitMetadataOnly_XmlDocs_DiagnoseDocMode_Success()
{
CSharpCompilation comp = CreateCompilation(@"
namespace Goo.Bar
{
/// <summary>This should emit</summary>
public class Test1
{
public static void SayHello()
{
Console.WriteLine(""hello"");
}
}
}
", assemblyName: "test", parseOptions: CSharpParseOptions.Default.WithDocumentationMode(DocumentationMode.Diagnose));

EmitResult emitResult;
byte[] mdOnlyImage;
byte[] xmlDocBytes;

using (var peStream = new MemoryStream())
using (var xmlStream = new MemoryStream())
{
emitResult = comp.Emit(peStream, xmlDocumentationStream: xmlStream, options: new EmitOptions(metadataOnly: true));
mdOnlyImage = peStream.ToArray();
xmlDocBytes = xmlStream.ToArray();
}

// This should not fail the emit (as it's a warning).
Assert.True(emitResult.Success);
emitResult.Diagnostics.Verify(
// (7,28): warning CS1591: Missing XML comment for publicly visible type or member 'Test1.SayHello()'
// public static void SayHello()
Diagnostic(ErrorCode.WRN_MissingXMLComment, "SayHello").WithArguments("Goo.Bar.Test1.SayHello()").WithLocation(7, 28));

Assert.True(mdOnlyImage.Length > 0, "no metadata emitted");
Assert.Equal(
"<?xml version=\"1.0\"?>\r\n<doc>\r\n <assembly>\r\n <name>test</name>\r\n </assembly>\r\n <members>\r\n <member name=\"T:Goo.Bar.Test1\">\r\n <summary>This should emit</summary>\r\n </member>\r\n </members>\r\n</doc>\r\n",
Encoding.UTF8.GetString(xmlDocBytes));
}

[Fact]
public void EmitMetadataOnly_XmlDocs_ParseDocMode_Success()
{
CSharpCompilation comp = CreateCompilation(@"
namespace Goo.Bar
{
/// <summary>This should emit</summary>
public class Test1
{
public static void SayHello()
{
Console.WriteLine(""hello"");
}
}
}
", assemblyName: "test", parseOptions: CSharpParseOptions.Default.WithDocumentationMode(DocumentationMode.Parse));

EmitResult emitResult;
byte[] mdOnlyImage;
byte[] xmlDocBytes;

using (var peStream = new MemoryStream())
using (var xmlStream = new MemoryStream())
{
emitResult = comp.Emit(peStream, xmlDocumentationStream: xmlStream, options: new EmitOptions(metadataOnly: true));
mdOnlyImage = peStream.ToArray();
xmlDocBytes = xmlStream.ToArray();
}

Assert.True(emitResult.Success);
emitResult.Diagnostics.Verify();

Assert.True(mdOnlyImage.Length > 0, "no metadata emitted");
Assert.Equal(
"<?xml version=\"1.0\"?>\r\n<doc>\r\n <assembly>\r\n <name>test</name>\r\n </assembly>\r\n <members>\r\n <member name=\"T:Goo.Bar.Test1\">\r\n <summary>This should emit</summary>\r\n </member>\r\n </members>\r\n</doc>\r\n",
Encoding.UTF8.GetString(xmlDocBytes));
}

[Fact]
public void EmitRefAssembly_PrivateMain()
{
Expand Down
11 changes: 3 additions & 8 deletions src/Compilers/Core/Portable/CommandLine/CommonCompiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1256,14 +1256,9 @@ private void CompileAndEmit(
return;
}

success = compilation.GenerateResourcesAndDocumentationComments(
moduleBeingBuilt,
xmlStreamDisposerOpt?.Stream,
win32ResourceStreamOpt,
useRawWin32Resources: false,
emitOptions.OutputNameOverride,
diagnostics,
cancellationToken);
success =
compilation.GenerateResources(moduleBeingBuilt, win32ResourceStreamOpt, useRawWin32Resources: false, diagnostics, cancellationToken) &&
compilation.GenerateDocumentationComments(xmlStreamDisposerOpt?.Stream, emitOptions.OutputNameOverride, diagnostics, cancellationToken);
}
}

Expand Down
34 changes: 21 additions & 13 deletions src/Compilers/Core/Portable/Compilation/Compilation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2237,14 +2237,22 @@ internal bool CreateDebugDocuments(DebugDocumentsBuilder documentsBuilder, IEnum
internal abstract void AddDebugSourceDocumentsForChecksumDirectives(DebugDocumentsBuilder documentsBuilder, SyntaxTree tree, DiagnosticBag diagnostics);

/// <summary>
/// Update resources and generate XML documentation comments.
/// Update resources.
/// </summary>
/// <returns>True if successful.</returns>
internal abstract bool GenerateResourcesAndDocumentationComments(
CommonPEModuleBuilder moduleBeingBuilt,
Stream? xmlDocumentationStream,
Stream? win32ResourcesStream,
internal abstract bool GenerateResources(
CommonPEModuleBuilder moduleBuilder,
Stream? win32Resources,
bool useRawWin32Resources,
DiagnosticBag diagnostics,
CancellationToken cancellationToken);

/// <summary>
/// Generate XML documentation comments.
/// </summary>
/// <returns>True if successful.</returns>
internal abstract bool GenerateDocumentationComments(
Stream? xmlDocStream,
string? outputNameOverride,
DiagnosticBag diagnostics,
CancellationToken cancellationToken);
Expand Down Expand Up @@ -2645,14 +2653,8 @@ internal EmitResult Emit(
{
// NOTE: We generate documentation even in presence of compile errors.
// https://github.com/dotnet/roslyn/issues/37996 tracks revisiting this behavior.
if (!GenerateResourcesAndDocumentationComments(
moduleBeingBuilt,
xmlDocumentationStream,
win32Resources,
useRawWin32Resources: rebuildData is object,
options.OutputNameOverride,
diagnostics,
cancellationToken))
if (!GenerateResources(moduleBeingBuilt, win32Resources, useRawWin32Resources: rebuildData is object, diagnostics, cancellationToken) ||
!GenerateDocumentationComments(xmlDocumentationStream, options.OutputNameOverride, diagnostics, cancellationToken))
{
success = false;
}
Expand All @@ -2662,6 +2664,12 @@ internal EmitResult Emit(
ReportUnusedImports(diagnostics, cancellationToken);
}
}
else if (xmlDocumentationStream != null)
{
// If we're in metadata only, and the caller asks for xml docs, then still proceed and generate those.
success = GenerateDocumentationComments(
xmlDocumentationStream, options.OutputNameOverride, diagnostics, cancellationToken);
}
}
finally
{
Expand Down
Loading