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

CS1587 Comment at start of file is not placed on a valid language element #663

Closed
ATECoder opened this issue Oct 23, 2020 · 3 comments
Closed
Labels
VB -> C# Specific to VB -> C# conversion

Comments

@ATECoder
Copy link

Input code

''' <summary> Form for viewing the my. </summary>
''' <remarks> David, 10/1/2020. </remarks>
Public Class MyForm
    Inherits isr.Automata.Finite.Forms.BimanualToggleForm
End Class

Erroneous output

/// <summary> Form for viewing the my. </summary>
/// <remarks> David, 10/1/2020. </remarks>

namespace isr.Finite.Automata.Engines
{
    public class MyForm : isr.Automata.Finite.Forms.BimanualToggleForm
    {
    }
}

Expected output

namespace isr.Finite.Automata.Engines
{
    /// <summary> Form for viewing the my. </summary>
    /// <remarks> David, 10/1/2020. </remarks>
    public class MyForm : isr.Automata.Finite.Forms.BimanualToggleForm
    {
    }
}

Details

  • Product in use: codeconverter.icsharpcode.net VS extension
  • Version in use: 8.1.8.0 CI artifact 10/12/20202
  • Did you see it working in a previous version, which? N/A
  • Any other relevant information to the issue, or your interest in contributing a fix. N/A
@ATECoder ATECoder added the VB -> C# Specific to VB -> C# conversion label Oct 23, 2020
@GrahamTheCoder GrahamTheCoder changed the title CS1587 XML comment is not placed on a valid language element CS1587 Comment at start of file is not placed on a valid language element Dec 24, 2020
@GrahamTheCoder
Copy link
Member

GrahamTheCoder commented Dec 24, 2020

The comment conversion works by using anchor points. e.g. The comment comes after the start of the file, and before the identifier "MySplashScreen".
All of the new lines generated for namespace/usings are in between converted version of the two anchors. At the moment, it picks the highest up line possible. Here's an example of a case where that would make sense, just by changing the contents of the comment:

Input code

''' This file is copyright notice must be preserved at the top of all files
Public Class MyForm
    Inherits isr.Automata.Finite.Forms.BimanualToggleForm
End Class

Current output

// This copyright notice must be preserved at the top of all files

namespace isr.Finite.Automata.Engines
{
    public class MyForm : isr.Automata.Finite.Forms.BimanualToggleForm
    {
    }
}

Output if I flip the heuristic

namespace isr.Finite.Automata.Engines
{
    // This copyright notice must be preserved at the top of all files
    public class MyForm : isr.Automata.Finite.Forms.BimanualToggleForm
    {
    }
}

Two resolutions that come to mind to solve at least these two cases:

  1. Flip the behaviour just for XML documentation comments
  2. Flip the behaviour everywhere except for comments at the start of a file which aren't XML doc comments

I'll probably try the second one and see how many tests it breaks, then retreat and have a rethink.

You could imagine going arbitrarily far adding heuristics for which line is relevant. e.g. When people have used ^ \/ and /\ to indicate which line they're referring to, or if there's a blank line before/after the comment. But it's a slippery slope leading to neural nets trained from the movement of comments in the history of all open source repositories, and I'm going to steer well clear of that as fun as it sounds 😄

@ATECoder
Copy link
Author

ATECoder commented Dec 24, 2020

Regular comments (i.e., starting with a " ' ") are not expected to raise a warning. It is code comments starting with
''' <summary> that raise the warnings.

in other words converting this:
''' This file is copyright notice must be preserved at the top of all files Public Class MyForm Inherits isr.Automata.Finite.Forms.BimanualToggleForm End Class

to this:

// This file is copyright notice must be preserved at the top of all files namespace isr.Finite.Automata.Engines { public class MyForm : isr.Automata.Finite.Forms.BimanualToggleForm { } }

is correct.

Converting this:
''' <summary> This file is copyright notice must be preserved at the top of all files </summary> Public Class MyForm Inherits isr.Automata.Finite.Forms.BimanualToggleForm End Class

to this:

/// <summary> This file is copyright notice must be preserved at the top of all files </summary> namespace isr.Finite.Automata.Engines { public class MyForm : isr.Automata.Finite.Forms.BimanualToggleForm { } }

will raise the warning.

@GrahamTheCoder
Copy link
Member

👍 Ah yes sorry that's what I meant by "doc comments" - I've now amended it to say "XML doc comments"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
VB -> C# Specific to VB -> C# conversion
Projects
None yet
Development

No branches or pull requests

2 participants