Skip to content

Commit

Permalink
[generator] Add support for XML documentation in the API definitions. F…
Browse files Browse the repository at this point in the history
…ixes #17397. (#20253)

* Enable generation of the documentation file (.xml) by the C# compiler when building projects (by passing the `DocumentationFlag` argument to the Csc task).
	* Also disable the CSC1591 warning, which complains about having public APIs without xml documentation; it just turns out to be annoying rather than helpful, since most APIs won't be documented.
* Add support in the generator for reading the .xml file next to the compiled api definition assembly, and then looking for documentation there when generating binding code.
	* When enabled, enable generation of the documentation file if the generator is compiling the api definitions.
	* Make it an opt-out, in case the new code causes problems.
* Add tests. 

Fixes #17397.

---------

Co-authored-by: Haritha Mohan <[email protected]>
  • Loading branch information
rolfbjarne and haritha-mohan committed Mar 13, 2024
1 parent 93ec62c commit f1d54e2
Show file tree
Hide file tree
Showing 10 changed files with 452 additions and 3 deletions.
8 changes: 6 additions & 2 deletions msbuild/Xamarin.Shared/Xamarin.Shared.targets
Original file line number Diff line number Diff line change
Expand Up @@ -1660,6 +1660,9 @@ Copyright (C) 2018 Microsoft. All rights reserved.

<PropertyGroup>
<_CompiledApiDefinitionAssembly>$(DeviceSpecificIntermediateOutputPath)compiled-api-definitions.dll</_CompiledApiDefinitionAssembly>
<_CompiledApiDefinitionNoWarn>$(_CompiledApiDefinitionNoWarn);436</_CompiledApiDefinitionNoWarn>
<_CompiledApiDefinitionDocumentationFile>$(DeviceSpecificIntermediateOutputPath)compiled-api-definitions.xml</_CompiledApiDefinitionDocumentationFile>
<_CompiledApiDefinitionNoWarn>$(_CompiledApiDefinitionNoWarn);1591</_CompiledApiDefinitionNoWarn> <!-- Don't expect every public API to have documentation -->
<_CompiledApiDefinitionDefines>$(DefineConstants)</_CompiledApiDefinitionDefines>
<_CompiledApiDefinitionDefines Condition="'$(UsingAppleNETSdk)' == 'true'">$(_CompiledApiDefinitionDefines);NET</_CompiledApiDefinitionDefines>

Expand Down Expand Up @@ -1697,7 +1700,7 @@ Copyright (C) 2018 Microsoft. All rights reserved.
$(_CompiledApiDefinitionGlobalUsingsFile);
@(_CompiledApiDefinitionReferences);
@(_CompiledApiDefinitionsCompile);"
Outputs="$(_CompiledApiDefinitionAssembly)"
Outputs="$(_CompiledApiDefinitionAssembly);$(_CompiledApiDefinitionDocumentationFile)"
DependsOnTargets="_ComputeCompileApiDefinitionsInputs"
Condition="'$(IsBindingProject)' == 'true' And '$(DesignTimeBuild)' != 'true'"
>
Expand All @@ -1708,7 +1711,8 @@ Copyright (C) 2018 Microsoft. All rights reserved.
AllowUnsafeBlocks="true"
DebugType="portable"
DefineConstants="$(_CompiledApiDefinitionDefines)"
DisabledWarnings="436"
DisabledWarnings="$(_CompiledApiDefinitionNoWarn)"
DocumentationFile="$(_CompiledApiDefinitionDocumentationFile)"
NoConfig="true"
NoStandardLib="true"
Deterministic="true"
Expand Down
22 changes: 22 additions & 0 deletions src/bgen/BindingTouch.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,15 @@ public class BindingTouch : IDisposable {
string []? compile_command = null;
string compiled_api_definition_assembly = string.Empty;
bool noNFloatUsing;
bool supportsXmlDocumentation = true;
List<string> references = new List<string> ();

public MetadataLoadContext? universe;
public Frameworks? Frameworks;

DocumentationManager? documentationManager;
public DocumentationManager DocumentationManager => documentationManager!;

AttributeManager? attributeManager;
public AttributeManager AttributeManager => attributeManager!;

Expand Down Expand Up @@ -229,6 +233,10 @@ public bool TryCreateOptionSet (BindingTouchConfig config, string [] args)
}
},
{ "compiled-api-definition-assembly=", "An assembly with the compiled api definitions.", (v) => compiled_api_definition_assembly = v },
{ "xmldoc:", "If the generator supports xml documentation in the API definition (default: true)", (v) => {
supportsXmlDocumentation = string.Equals ("true", v, StringComparison.OrdinalIgnoreCase) || string.IsNullOrEmpty (v);
}
},
new Mono.Options.ResponseFileSource (),
};
config.Sources = config.OptionSet.Parse (args);
Expand Down Expand Up @@ -282,6 +290,8 @@ public bool TryInitializeApi (BindingTouchConfig config, [NotNullWhen (true)] ou
!TryLoadApi (LibraryInfo.BaseLibDll, out Assembly? baselib))
return false;

documentationManager = new DocumentationManager (supportsXmlDocumentation ? tmpass : string.Empty);

Frameworks = new Frameworks (CurrentPlatform);

// Explicitly load our attribute library so that IKVM doesn't try (and fail) to find it.
Expand Down Expand Up @@ -396,6 +406,12 @@ bool TryGenerate (BindingTouchConfig config, Api api)
}
if (!string.IsNullOrEmpty (Path.GetDirectoryName (LibraryInfo.BaseLibDll)))
cargs.Add ("-lib:" + Path.GetDirectoryName (LibraryInfo.BaseLibDll));
if (supportsXmlDocumentation) {
cargs.Add ("-doc:" + Path.ChangeExtension (outfile, ".xml"));
// warning CS1591: Missing XML comment for publicly visible type or member
// Ignore it, because we don't expect code to have everything documented.
cargs.Add ("-nowarn:1591");
}

AddNFloatUsing (cargs, config.TemporaryFileDirectory);

Expand Down Expand Up @@ -463,6 +479,12 @@ string GetCompiledApiBindingsAssembly (LibraryInfo libraryInfo, BindingTouchConf
cargs.AddRange (core_sources);
if (!string.IsNullOrEmpty (Path.GetDirectoryName (libraryInfo.BaseLibDll)))
cargs.Add ("-lib:" + Path.GetDirectoryName (libraryInfo.BaseLibDll));
if (supportsXmlDocumentation) {
cargs.Add ("-doc:" + Path.ChangeExtension (tmpass, ".xml"));
// warning CS1591: Missing XML comment for publicly visible type or member
// Ignore it, because we don't expect code to have everything documented.
cargs.Add ("-nowarn:1591");
}

AddNFloatUsing (cargs, tmpdir);

Expand Down
180 changes: 180 additions & 0 deletions src/bgen/DocumentationManager.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Linq;
using System.Reflection;
using System.Text;
using System.Xml;

#nullable enable

public class DocumentationManager {
string xml;
XmlDocument? doc;

public DocumentationManager (string assembly)
{
this.xml = Path.ChangeExtension (assembly, ".xml");
if (File.Exists (xml)) {
doc = new XmlDocument ();
doc.LoadWithoutNetworkAccess (xml);
}
}

public void WriteDocumentation (StreamWriter sw, int indent, MemberInfo member)
{
if (!TryGetDocumentation (member, out var docs))
return;

foreach (var line in docs) {
sw.Write ('\t', indent);
sw.WriteLine (line);
}
}

public bool TryGetDocumentation (MemberInfo member, [NotNullWhen (true)] out string []? documentation)
{
documentation = null;

if (doc is null)
return false;

if (!TryGetId (member, out var id))
return false;

var node = doc.SelectSingleNode ($"/doc/members/member[@name='{id}']");
if (node is null)
return false;

// Remove indentation, make triple-slash comments
var lines = node.InnerXml.Split ('\n', '\r');
for (var i = 0; i < lines.Length; i++) {
lines [i] = "/// " + lines [i].TrimStart (' ');
}

documentation = lines;

return true;
}

// https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/xmldoc/#id-strings
// See tests/cecil-tests/Documentation.cs for an implementation that works with Cecil.
// There's already an implementation in Roslyn, but that's a rather heavy dependency,
// so we're implementing this in our own code instead.

static string GetDocId (MethodInfo md)
{
var methodName = md.Name.Replace ('.', '#');
var name = GetDocId (md.DeclaringType!) + "." + methodName;
if (md.IsGenericMethodDefinition)
name += $"``{md.GetGenericArguments ().Length}";
var parameters = md.GetParameters ();
if (parameters.Length > 0) {
name += "(" + string.Join (",", parameters.Select (p => GetDocId (p.ParameterType))) + ")";
}

if (md.Name == "op_Explicit" || md.Name == "op_Implicit") {
name += "~" + GetDocId (md.ReturnType);
}

return name;
}

static string GetDocId (EventInfo ed) => GetDocId (ed.DeclaringType!) + "." + ed.Name;

static string GetDocId (PropertyInfo pd)
{
var name = GetDocId (pd.DeclaringType!) + "." + pd.Name;
var parameters = pd.GetIndexParameters ();
if (parameters.Length > 0) {
name += "(" + string.Join (",", parameters.Select (p => GetDocId (p.ParameterType))) + ")";
}
return name;
}


static bool TryGetId (MemberInfo member, [NotNullWhen (true)] out string? name)
{
name = null;

if (member is MethodInfo md) {
name = "M:" + GetDocId (md);
} else if (member is PropertyInfo pd) {
name = "P:" + GetDocId (pd);
} else if (member is FieldInfo fd) {
name = "F:" + GetDocId (fd.DeclaringType!) + "." + fd.Name;
} else if (member is EventInfo ed) {
name = "E:" + GetDocId (ed);
} else if (member is Type td) {
name = "T:" + GetDocId (td);
} else {
return false;
}
return true;
}

static string GetDocId (Type tr)
{
var name = new StringBuilder ();

if (tr.IsGenericParameter) {
name.Append ('`');
name.Append (tr.GenericParameterPosition);
#if NET
} else if (tr.IsSZArray) {
#else
} else if (tr.IsArray && tr.GetArrayRank () == 1) { // Not quite the same as IsSZArray (see https://github.com/dotnet/runtime/issues/20376), but good enough for legacy.
#endif
name.Append (GetDocId (tr.GetElementType ()!));
name.Append ("[]");
} else if (tr.IsArray) {
// As far as I can tell, System.Reflection doesn't provide a way to get the dimensions (lower/upper bounds) of the array type.
// That said, C# doesn't provide a way to set them either, so this should work for xml documentation produced by C# at least.
name.Append (GetDocId (tr.GetElementType ()!));
name.Append ('[');
for (var i = 0; i < tr.GetArrayRank (); i++) {
if (i > 0)
name.Append (',');
name.Append ("0:"); // C# always produces multidimensional arrays with lower bound = 0 and no upper bound.
}
name.Append (']');
} else {
if (tr.IsNested) {
var decl = tr.DeclaringType!;
while (decl.IsNested) {
name.Append (decl.Name);
name.Append ('.');
name.Append (name);
decl = decl.DeclaringType!;
}
name.Insert (0, '.');
name.Insert (0, decl.Namespace);
} else {
name.Append (tr.Namespace);
name.Append ('.');
}

if (tr.IsGenericTypeDefinition) {
name.Append (tr.Name);
} else if (tr.IsGenericType) {
name.Append (tr.Name, 0, tr.Name.IndexOf ('`'));
name.Append ('{');
var genericArguments = tr.GetGenericArguments ();
for (var i = 0; i < genericArguments.Length; i++) {
if (i > 0)
name.Append (',');
name.Append (GetDocId (genericArguments [i]));
}
name.Append ('}');
} else if (tr.IsByRef) {
name.Append (tr.GetElementType ()!.Name);
name.Append ('@');
} else {
name.Append (tr.Name);
}
}

return name.ToString ();
}
}
14 changes: 14 additions & 0 deletions src/bgen/Extensions/ExtensionMethods.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
using System.IO;
using System.Linq;
using System.Reflection;
using System.Xml;

using Foundation;
using ObjCRuntime;

Expand All @@ -18,6 +20,18 @@ public static StreamWriter Write (this StreamWriter sw, char c, int count)
return sw;
}

public static void LoadWithoutNetworkAccess (this XmlDocument doc, string filename)
{
using (var fs = new FileStream (filename, FileMode.Open, FileAccess.Read)) {
var settings = new XmlReaderSettings () {
XmlResolver = null,
DtdProcessing = DtdProcessing.Parse,
};
using (var reader = XmlReader.Create (fs, settings)) {
doc.Load (reader);
}
}
}
}

public static class ReflectionExtensions {
Expand Down
20 changes: 20 additions & 0 deletions src/bgen/Generator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ public partial class Generator : IMemberGatherer {
public AttributeManager AttributeManager { get { return BindingTouch.AttributeManager; } }
NamespaceCache NamespaceCache { get { return BindingTouch.NamespaceCache; } }
public TypeCache TypeCache { get { return BindingTouch.TypeCache; } }
public DocumentationManager DocumentationManager { get { return BindingTouch.DocumentationManager; } }

Nomenclator nomenclator;
Nomenclator Nomenclator {
Expand Down Expand Up @@ -3841,6 +3842,8 @@ void GenerateProperty (Type type, PropertyInfo pi, List<string> instance_fields_
}
}

WriteDocumentation (pi);

if (wrap is not null) {
print_generated_code ();
PrintPropertyAttributes (pi, minfo.type);
Expand Down Expand Up @@ -4392,6 +4395,12 @@ void GenerateMethod (MemberInformation minfo)
}
}

if (minfo.is_extension_method) {
WriteDocumentation ((MemberInfo) GetProperty (minfo.Method) ?? minfo.Method);
} else {
WriteDocumentation (minfo.Method);
}

PrintDelegateProxy (minfo);

if (AttributeManager.HasAttribute<NoMethodAttribute> (minfo.mi)) {
Expand Down Expand Up @@ -4673,6 +4682,8 @@ void GenerateProtocolTypes (Type type, string class_visibility, string TypeName,
var optionalInstanceProperties = allProtocolProperties.Where ((v) => !IsRequired (v) && !AttributeManager.HasAttribute<StaticAttribute> (v));
var requiredInstanceAsyncMethods = requiredInstanceMethods.Where (m => AttributeManager.HasAttribute<AsyncAttribute> (m)).ToList ();

WriteDocumentation (type);

PrintAttributes (type, platform: true, preserve: true, advice: true);
print ("[Protocol (Name = \"{1}\", WrapperType = typeof ({0}Wrapper){2}{3})]",
TypeName,
Expand Down Expand Up @@ -4812,6 +4823,7 @@ void GenerateProtocolTypes (Type type, string class_visibility, string TypeName,
var minfo = new MemberInformation (this, this, mi, type, null);
var mod = string.Empty;

WriteDocumentation (mi);
PrintMethodAttributes (minfo);
print_generated_code ();
PrintDelegateProxy (minfo);
Expand All @@ -4828,6 +4840,7 @@ void GenerateProtocolTypes (Type type, string class_visibility, string TypeName,
var mod = string.Empty;
minfo.is_export = true;

WriteDocumentation (pi);
print ("[Preserve (Conditional = true)]");
PrintAttributes (pi, platform: true);

Expand Down Expand Up @@ -5203,6 +5216,11 @@ public void PrintAttributes (ICustomAttributeProvider mi, bool platform = false,
PrintRequiresSuperAttribute (mi);
}

void WriteDocumentation (MemberInfo info)
{
DocumentationManager.WriteDocumentation (sw, indent, info);
}

public void ComputeLibraryName (FieldAttribute fieldAttr, Type type, string propertyName, out string library_name, out string library_path)
{
library_path = null;
Expand Down Expand Up @@ -5362,6 +5380,8 @@ public void Generate (Type type)
indent++;
}

WriteDocumentation (type);

bool core_image_filter = false;
string class_mod = null;

Expand Down
2 changes: 2 additions & 0 deletions src/generator.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
<ItemGroup>
<Reference Include="System" />
<Reference Include="System.Core" />
<Reference Include="System.Xml" />
<PackageReference Include="Mono.Options" Version="6.12.0.148" />
<PackageReference Include="System.Reflection.MetadataLoadContext" Version="4.7.2" />
</ItemGroup>
Expand Down Expand Up @@ -103,6 +104,7 @@
<Compile Include="..\src\bgen\AttributeFactory.ConstructorArguments.cs" />
<Compile Include="..\src\bgen\AttributeFactory.cs" />
<Compile Include="..\src\bgen\BindingTouch.cs" />
<Compile Include="..\src\bgen\DocumentationManager.cs" />
<Compile Include="..\src\bgen\Enums.cs" />
<Compile Include="..\src\bgen\Frameworks.cs" />
<Compile Include="..\src\bgen\Filters.cs" />
Expand Down
Loading

8 comments on commit f1d54e2

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

Please sign in to comment.