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

Confusing code generated for XML elements in documentation comments #47

Open
svick opened this issue Apr 3, 2019 · 2 comments
Open

Comments

@svick
Copy link
Contributor

svick commented Apr 3, 2019

Consider a member with XML documentation comment:

/// <summary></summary>
int i;

The relevant section of code generated for the XML comment is:

XmlExampleElement()
.WithStartTag(
    XmlElementStartTag(
        XmlName(
            Identifier("summary"))))
.WithEndTag(
    XmlElementEndTag(
        XmlName(
            Identifier("summary"))))

Notice how the code first creates an <example> element and then overwrites both tags with the right name. That's not technically wrong, but it's certainly a confusing way to create the tree.

Ideally, RoslynQuoter should use a specific factory method, when one is available (in this case, XmlSummaryElement). Though always using the generic XmlElement method would be fine too.

@KirillOsenkov
Copy link
Owner

Unfortunately the algorithm breaks down when there's more than one way to create the given tree. It works so well in general because usually there's only a single method that can create a node of the desired type, so there's no ambiguity.

However Xml factory methods were added later, and they have a few convenience methods, so the algorithm just chooses the first one that works, and that explains the results above.

We'd need some heuristics to fine tune this behavior and provide hints to the algorithm on a case-by-case basis.

@KirillOsenkov
Copy link
Owner

See here for details:

// In this commit: https:/dotnet/roslyn/commit/4c19f1b28df66eaf3035105ec5b8bb35bfeb6869

I'm not sure how to add such a heuristic, perhaps we can have "factoryMethodsToPrefer", so we can disambiguate here:

factory = candidatesWithMinParameterCount[0];

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants