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

Inappropriate usages of tags for creating a well-formed XML file in C# #843

Closed
7 tasks done
BethanyZhou opened this issue Sep 28, 2021 · 0 comments · Fixed by Azure/azure-powershell#18117 or #939
Closed
7 tasks done
Labels

Comments

@BethanyZhou
Copy link
Contributor

BethanyZhou commented Sep 28, 2021

Inappropriate usages of tags for creating a well-formed XML file in C#

  • Comments in c# code
  • Using <param> in a property
  • Wrong place of XML comment
  • Referring an array
  • Referring a parameter as a type
  • Incomplete tag
  • Mix scenario

Comments in c# code

Example

public async Task EventListener(string id, CancellationToken cancellationToken, GetEventData getEventData, SignalDelegate signal, InvocationInfo invocationInfo, string parameterSetName, string correlationId, string processRecordId, System.Exception exception)
{
      /// Drain the queue of ADAL events whenever an event is fired
      DrainDeferredEvents(signal, cancellationToken);
      ...
}

Issue

/// is XML tags comments, which should follow with tags

Solution

/// Drain the queue of ADAL events whenever an event is fired
->
// Drain the queue of ADAL events whenever an event is fired

Using <param> in a property

Example

/// <summary>
/// The cmdlet will call this when it is trying to fill in a parameter value that it needs
/// </summary>
/// <param name="resourceId"><c>string</c>containing the expected resource id (ie, ARM).</param>
/// <param name="moduleName"><c>string</c>containing the name of the module being loaded.</param>
/// <param name="invocationInfo">The <see cref="System.Management.Automation.InvocationInfo" /> from the cmdlet</param>
/// <param name="correlationId">The <see cref="string" /> containing the correlation id for the cmdlet</param>
/// <param name="name">The <see cref="string" /> parameter name being asked for</param>
public GetParameterDelegate GetParameterValue;

Solution

The tag should be used in the comment for a method declaration to describe one of the parameters for the method. We shouldn't use in property. Remove them

/// <summary>
/// The cmdlet will call this when it is trying to fill in a parameter value that it needs
/// </summary>
public GetParameterDelegate GetParameterValue;

Wrong place of XML comment

Example

[TypeConverter(typeof(EventDataConverter))]
/// <remarks>
/// In PowerShell, we add on the EventDataConverter to support sending events between modules.
/// Obviously, this code would need to be duplcated on both modules.
/// This is preferable to sharing a common library, as versioning makes that problematic.
/// </remarks>
public partial class EventData : EventArgs
{
    ...
}

Solution

Switch the position of comments and attributes

/// <remarks>
/// In PowerShell, we add on the EventDataConverter to support sending events between modules.
/// Obviously, this code would need to be duplcated on both modules.
/// This is preferable to sharing a common library, as versioning makes that problematic.
/// </remarks>
[TypeConverter(typeof(EventDataConverter))]
public partial class EventData : EventArgs
{
    ...
}

Referring an array

Example

/// <param name="resourceTypes">An <see cref="System.String[]"/> containing resource (or resource types) being completed  </param >

Issue

specifies that the inner text of the tag is a code element, such as a type, method, or property. string[] is not a generic type. string is.

Solution

/// <param name="resourceTypes">An <see cref="System.String[]"/> containing resource (or resource types) being completed  </param >
->
/// <param name="resourceTypes">An <see cref="string"/>[] containing resource (or resource types) being completed  </param >

Referring a parameter as a type

Example

/// <param name="e1">the value to compare against <see cref="e2" /></param>
/// <param name="e2">the value to compare against <see cref="e1" /></param>
public static bool operator ==(Microsoft.Azure.PowerShell.Cmdlets.ADDomainServices.Support.ExternalAccess e1, Microsoft.Azure.PowerShell.Cmdlets.ADDomainServices.Support.ExternalAccess e2)
{
     return e2.Equals(e1);
}

Issue

specifies that the inner text of the tag is a code element, such as a type, method, or property. e2/e1 is not a type. We should use .

Solution

<see cref="e2" /> -> <paramref name="e2" /></param>
<see cref="e1" /> -> <paramref name="e1" /></param>

Incomplete tag

Example

/// <summary>Creates an instance of the <see cref="ExternalAccess" Enum class./></summary>

Issue

Complete tag should be or Enum class.

Solution

/// <summary>Creates an instance of the <see cref="ExternalAccess"> Enum class.</see></summary>

Mix scenario

Example

/// <summary>
/// <c>BeforeFromJson</c> will be called before the json deserialization has commenced, allowing complete customization of
/// the object before it is deserialized.
/// If you wish to disable the default deserialization entirely, return <c>true</c> in the <see "returnNow" /> output parameter.
/// Implement this method in a partial class to enable this behavior.
/// </summary>
/// <param name="json">The JsonNode that should be deserialized into this object.</param>
/// <param name="returnNow">Determines if the rest of the deserialization should be processed, or if the method should return
/// instantly.</param>
partial void BeforeFromJson(Microsoft.Azure.PowerShell.Cmdlets.ADDomainServices.Runtime.Json.JsonObject json, ref bool returnNow);

Issue

  • Complete tag should be <see cref="returnNow"/>
  • "returnNow" is parameter, its reference should use <paramref name="returnNow" />

Solution

<see "returnNow" />
->
<paramref name="returnNow" />
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants