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

Inconsistent indenting displaying for XML doc comments between base member and derived (impl'ed) member #67180

Closed
KyouyamaKazusa0805 opened this issue Mar 4, 2023 · 4 comments · Fixed by #71589
Labels
Area-IDE Bug good first issue The issue is reserved for a first time, non-Microsoft contributor Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented
Milestone

Comments

@KyouyamaKazusa0805
Copy link

KyouyamaKazusa0805 commented Mar 4, 2023

Version Used:

VS 17.6p1

Steps to Reproduce:

Define an interface member and append doc comments:

interface I
{
    /// <summary>
    /// A property.
    /// </summary>
    /// <remarks>
    /// <para>
    /// You can use this property like:
    /// <code><![CDATA[
    /// var result = Property;
    /// ]]></code>
    /// </para>
    /// </remarks>
    int Property { get; }
}

And then define a class that implements this property, and use <inheritdoc/> to inherit its doc comments:

class C : I
{
    /// <inheritdoc/>
    public int Property => 42;
}

Expected Behavior:

Both doc comments displayed in tooltip should be totally same.

Actual Behavior:

In derived (implemented) type, an unexpected indent will be inserted into the doc comments displayed in tooltip.

Screenshot:

Interface type:

image

Implemented type:

image

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Mar 4, 2023
@mavasani mavasani added Bug help wanted The issue is "up for grabs" - add a comment if you are interested in working on it good first issue The issue is reserved for a first time, non-Microsoft contributor and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Oct 19, 2023
@mavasani mavasani added this to the Backlog milestone Oct 19, 2023
@mavasani
Copy link
Contributor

@sharwell

@ghost
Copy link

ghost commented Jan 11, 2024

@mavasani Hey, can I take this issue?

@sharwell
Copy link
Member

@sdelarosbil Sure 👍

@ghost
Copy link

ghost commented Jan 11, 2024

@sharwell Just giving a quick update on this: I believe I have found the cause of the issue and will be submitting a pr shortly. The fix seems straight forward, but the concern I have is what lead me to uncover this sounds like a different issue manifesting itseflf.

The issue boils down to an XDocument.Parse call not having a LoadOptions.PreserveWhitespace during the inheritdoc rewrite. This means that the following (which is enough to reproduce still):

/// <summary>
/// <code>
/// var test;
/// </code></summary>

Actually gets rewritten to this when an inheritdoc element is present:

/// <summary><code>
/// var test;
/// </code></summary>

This is due to not preserving the whitespaces of summary's child nodes. Semantically, these 2 should be equivalent, but this is what causes the indenting change which suggests there's a different problem: why would the newline's absence change the indentation?

Even if I give this the benefit of the doubt and assume it's done on purpose: it behaves in the opposite manner that I expect. It turns out the first snipped produces no trailing nor leading whitespaces while the second snipped produces \n var test;\n . The leading LF gets trimmed due to the code element's parsing logic, but the trailing LF and other spaces are still kept. If anything, I would have expected the first snippet to put an LF if any were supposed to be there, but it's actually the second one.

I don't think this finding changes the original fix as loosing the whitespaces when loading inheritdoc could lead to other problems and the whole pipeline seem to preserve them as much as possible anyway. What I am concerned about is if there should be another issue opened about this odd indentation behavior in the first place.

@sharwell sharwell modified the milestones: Backlog, Next Jan 17, 2024
@sharwell sharwell added Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented and removed help wanted The issue is "up for grabs" - add a comment if you are interested in working on it labels Jan 17, 2024
@Cosifne Cosifne modified the milestones: Next, 17.10 P1 Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Bug good first issue The issue is reserved for a first time, non-Microsoft contributor Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants