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

Directives make Roslyn Diagnostic Locations incorrect #84

Closed
andersforsgren opened this issue Dec 10, 2019 · 3 comments
Closed

Directives make Roslyn Diagnostic Locations incorrect #84

andersforsgren opened this issue Dec 10, 2019 · 3 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@andersforsgren
Copy link
Contributor

andersforsgren commented Dec 10, 2019

Steps to reproduce

A rule source with

.reference="Lib"    // line 1
return "Hello world"    // line 2 (With missing semicolon)

Will produce a roslyn diagnostic for the missing semicolon, but with location on Line 1, because the directive line is stripped before the remaining lines are compiled to a csharpscript.

Expected behavior

The compiler diagnostic in the above example is for Line 2.
That is, roslyn diagnostics have locations that make sense in the overall buffer including directives, because that is the buffer the user is likely to edit in a user interface.

Actual behavior

Roslyn diagnostics have locations that are only correct in the buffer after the directive lines with leading dots are removed.

I speculate that this could be easy to fix by simply including the directive lines commented out to the CSharpScript, instead of removing them. So instead of sending this array of 1 line

return Hello world

This array of 2 lines should be the Script source:

//.reference="Lib"
return Hello world 

I'll attempt to make a PR with this solution if it looks acceptable.

Diagnostic logs

Environment

v0.9.2-35-ga24ff33

@giuliov giuliov added enhancement New feature or request good first issue Good for newcomers pinned Stops stale[bot] from closing this issue labels Dec 10, 2019
@giuliov
Copy link
Member

giuliov commented Dec 10, 2019

Nice catch. Added item to backlog through labels

@andersforsgren
Copy link
Contributor Author

I'll submit a PR for this shortly

@giuliov giuliov removed the pinned Stops stale[bot] from closing this issue label Dec 26, 2019
@giuliov
Copy link
Member

giuliov commented Dec 26, 2019

Included in v0.9.8.

@giuliov giuliov closed this as completed Dec 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants