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

VB Editor changed VB keywords casing although they are used as literal strings in my code! #35028

Closed
VBAndCs opened this issue Apr 17, 2019 · 56 comments
Labels
Area-IDE Bug help wanted The issue is "up for grabs" - add a comment if you are interested in working on it IDE-Formatter Code formatter and/or smart indent
Milestone

Comments

@VBAndCs
Copy link

VBAndCs commented Apr 17, 2019

Version Used:
VS.NET 2019 Community

I had this Function in VB

    Private Function convVars(type As String) As String
        If type Is Nothing Then Return Nothing
        Dim t = type.Trim().ToLower()
        Select Case t
            Case "Byte", "SByte", "Short", "UShort", "Long", "ULong", "douple", "Decimal"
                Return t
            Case "Integer"
                Return "int"
            Case "UInteger"
                Return "uint"
            Case "Single"
                Return "float"
            Case Else
                Return type.Trim().Replace(
                    (" Byte", " Byte"), (" SBtye", " SByte"), (" Short", " Short"),
                    (" UShort", " UShort"), (" Long", " Long"), (" ULong", " ULong"),
                    (" Double", " douple"), (" Decimal", " Decimal"),
                    (" Integer", " int"), (" UInteger", " uint"), (" Single", " float"),
                    ("(Of ", LessThan), ("Of ", LessThan), (")", GreaterThan)
                )

        End Select


    End Function

You can notice the statement:
Dim t = type.Trim().ToLower()
Then I check the values of t using Select case
Obviously, I wouldn't change to small case the compare with strings containing upper case letters!
This code isn't what I wrote! The editor suddenly changed the case of all VB keywords although they are just quoted strings!
This happened also in another function where I used the string "declare" but suddenly found it became "Declare"!
Also I found a strange Then added after a string although there is no If nearby!
The sudden failure of unit testes (that passed before) reveals all this!
I had to repair all these editor mistakes, but I am afraid to happen again!
I don't know what triggers this strange action. I had these codes running for days! This just happened in the past few minutes, and I have no reason to think about!

Note:
It seems related to XML literals. I explained how to reproduce this in #35028 (comment)

@VBAndCs
Copy link
Author

VBAndCs commented Apr 17, 2019

Note: the only word that didnt change is douple, because it has a typo! I missed this in testing and would go unnoticed except for this faital editor error :)

@VBAndCs
Copy link
Author

VBAndCs commented Apr 17, 2019

And this is a third place:

        Dim getter = (From elm In Xml.Descendants()
                      Where elm.Name = "Get").FirstOrDefault

It was "get" but changedd to "Get"!

@VBAndCs
Copy link
Author

VBAndCs commented Apr 17, 2019

A fourth one:

            Dim obj = At(If(getter.Attribute("Object")?.Value, getter.Value))

It was "object" but changed to "Object". I am still fixing the test errors!

@VBAndCs
Copy link
Author

VBAndCs commented Apr 17, 2019

Same here

        Dim setter = (From elm In Xml.Descendants()
                      Where elm.Name = "Set").FirstOrDefault

and here:

Dim obj = setter.Attribute("Object")

@VBAndCs
Copy link
Author

VBAndCs commented Apr 17, 2019

I fixed those, and updated the repo:
https://github.com/vbAndCs/vazor-DotNetCore2/

The solution that cuased this is:
https://github.com/VBAndCs/Vazor-DotNetCore2/blob/master/VazorTest/VazorTest.sln

it contains the ZML.cs in the Vazor app. This class contains those VB lower cass keywords.
I can't tell how to reproduce this action, but you should check how VB editor check for keywords and change them to upper case, and if it can do that regardless of the string quotes.

@CyrusNajmabadi
Copy link
Member

it contains the ZML.cs in the Vazor app. This class contains those VB lower cass keywords.

We do not change code inside strings. However, what may have happened to you is that you changed a quote somewhere in your file, causing your strings/non-strings to become inverted. You then performed an action that triggers cleanup (like saving the file) and the keywords in non-string areas got case corrected.

I can't tell how to reproduce this action, but you should check how VB editor check for keywords and change them to upper case, and if it can do that regardless of the string quotes.

Without a clear set of repro steps, it's unlikely anything can be done here. As it stands, it's likely that you were editing and left the code in a state where VB itself thought your strings/non-strings were flipped. IN that case, this would be correcting the appropriate sections of your code.

As such, this is all likely by design. If you can get repro steps that shows that actual strings were changes, feel free to reopen with that information. Thanks!

@VBAndCs
Copy link
Author

VBAndCs commented Apr 17, 2019

@CyrusNajmabadi
Your explanation is reasonable, but this behavior is scary!
Since VB allowed multi line strings, the whole file could be messed up just because a missed quote!
I suggest you do something about that. At least preserve function bounds, so that the editor re-parse code only within the smallest code block that has the change. I and others complained that the editor is slow, may be because this. It has to check the whole file with every key stroke. Some times I had to close the file and reopen it because the editor can't infer some var type!
I expect more of these errors to come. There is a problem with the editor in this version, so please test it.

@CyrusNajmabadi
Copy link
Member

Since VB allowed multi line strings, the whole file could be messed up just because a missed quote!

That's what undo is for.

I suggest you do something about that. At least preserve function bounds, so that the editor re-parse code only within the smallest code block that has the change

The editor reparses what is necessary to construct a full parse tree from the code you have written.

I and others complained that the editor is slow, may be because this

If you feel the editor is slow, please file feedback in VS. You can use the information here on how to do is: https://github.com/dotnet/roslyn/wiki/Reporting-Visual-Studio-crashes-and-performance-issues

It's pointless to speculate as slowness could be any number of issues.

It has to check the whole file with every key stroke.

Yes. This is how C# and VB and TypeSript and F# and all the other languages work. As you type, the file is continually rechecked to see if there are issues.

Some times I had to close the file and reopen it because the editor can't infer some var type!

I don't see the relation between the issues.

There is a problem with the editor in this version, so please test it.

You have to supply a realistic repro that allows people to determine what the root cause of hte issue is. Right now, it might just be user error on your part. Roslyn currently has tens of thousands of tests (if not more). Something may have slipped through, but asking people to "test it" isn't going to solve anything since it's absolutely not clear at all what went wrong or what is at fault.

@VBAndCs
Copy link
Author

VBAndCs commented Apr 17, 2019

As you type, the file is continually rechecked to see if there are issues.

Let's focus on that. This should be done when the file opened and at build time. Suppose I opened a class file, and the editor checked it and it is OK.
Now I am editing code inside one method B in the class. The editor can check syntax within the scope of this function, since it is sure that method A and Method C are OK. It needs to save the start and end pos of each method. Editing will change those positions, but it is easy to use one offset var that can increase or decrease while editing method B, so, the start Pos of method C can be calculated by adding this offset. By this simple way, the editor will be faster, and the code in other methods will be protected against editor's misbehaviors. The editor don't need re-parse other methods except after a replacement operation in the scope of the document or the bigger. Paste some text inside the method should be ok, even I pasted a whole class inside the method. It is all wrong and no need to check other methods to prove that! Unless the paste is a total replacement of the method itself, so the file must be re-parsed.
I think the editor can be optimized. I am not OK with having many methods altered away from my eyes, just because I wrote or erased a semicolon! The editor should not change any thing behind my back, otherwise I violates the trust between us.

@CyrusNajmabadi
Copy link
Member

Now I am editing code inside one method B in the class. The editor can check syntax within the scope of this function, since it is sure that method A and Method C are OK. It needs to save the start and end pos of each method. Editing will change those positions, but it is easy to use one offset var that can increase or decrease while editing method B, so, the start Pos of method C can be calculated by adding this offset

As i said: The editor reparses what is necessary

@CyrusNajmabadi
Copy link
Member

By this simple way, the editor will be faster, and the code in other methods will be protected against editor's misbehaviors.

Feel free to contribute any improvements you think will make things better.

The editor should not change any thing behind my back, otherwise I violates the trust between us.

Feel free to disable this feature (which VB has had since the beginning):

image

@VBAndCs
Copy link
Author

VBAndCs commented Apr 17, 2019

I would better want the editor to stop formatting the file while it contains syntax errors. This will prevent this case changing issue prevent and adding then and other block endings in strange places.
It is a one simple fail safe condition. Can it be done please?

@jmarolf
Copy link
Contributor

jmarolf commented Apr 17, 2019

I would better want the editor to stop formatting the file while it contains syntax errors.

This is interesting. That would mean a single error in a file would block formatting unnecessarily for the entire file.

I think we've gotten a little in the weeds here regarding implementation details. @VBAndCs it sounds like what you want is for the formatter to be resilient to error cases around incorrectly escaped strings. This way formatting works for the entire file but doesn't format the area around the string. I still don't know what the correct case for identifying this syntactically would be, but we can figure that out later :)

@VBAndCs
Copy link
Author

VBAndCs commented Apr 17, 2019

@jmarolf
If the existence of keywords in literal strings is the only reason of this confusion, the simple solution is to define them as constants in a separate file. This is what I did manfully (by the way, I wish if there is a context menu command to replace the selected literal with a generated constant holding this literal), and VB editor can do behind the scene. These constants should updated only if there string literal is edited. In fact substituting all strings literals with placeholders (constants, or even an ID string) can make things easier for the editor. Suppose there is a placeholder like this: "_stringplaceholder0000000001" and a missing quote inverses things so the editor sees this name as a code entity. The editor will easily realize that stringplaceholder0000000001 is an auto generated ID and can't be a code token, so, it realizes that there is something wrong and corrects itself. To achieve this, the editor may keep the last parse tree as a reference. I have no experience in this area, so, excuse me if these are not applicable ideas.

@jmarolf
Copy link
Contributor

jmarolf commented Apr 17, 2019

If the existence of keywords in literal strings is the only reason of this confusion

Good point, more specifically we don't want to formatter to change capitalization around string literals.

@CyrusNajmabadi
Copy link
Member

I would better want the editor to stop formatting the file while it contains syntax errors.

This would likely be very unpleasant. 99.9% of the time a file has syntax errors, and we depend on formatting so that you can do things like hit enter and have everything properly aligned and whatnot.

It is a one simple fail safe condition. Can it be done please?

If you create a PR we can discuss and consider the implications of the change.

the simple solution is to define them as constants in a separate file. This is what I did manfully (by the way, I wish if there is a context menu command to replace the selected literal with a generated constant holding this literal),

You can do this today:

image

@VBAndCs
Copy link
Author

VBAndCs commented Apr 17, 2019

we depend on formatting so that you can do things

  1. If you have an error-free file it is formated.
  2. Now if you deleted 1 quote that destroys the whole file structure, then all what I am asking is leaving the file as it is. It is already formated (unless at the editing position)
  3. The errors are listed, and I can fix them.
  4. Now the editor can recheck the file format.

@CyrusNajmabadi
Copy link
Member

@VBAndCs please provide a repo of the issue you are running into.

@VBAndCs
Copy link
Author

VBAndCs commented Apr 18, 2019

@CyrusNajmabadi
I tried, but can't reproduce this issue in a test sample. The editor doesn't change the casing of literal keywords even they appear in blue color red!
Maybe this is an exotic case related to my zml parser that contains a lot of vb keywords as a literals. I did all I can to prevent it from happining again by using constants, so, my original code has changed and can't reproduce the same situation.
If I faced a similar case, I will report it with more info.
Thanks.

@VBAndCs
Copy link
Author

VBAndCs commented Apr 18, 2019

I can think of one reason:
Sometimes when I open the project with number of code files opened, VB compiler takes too long time to check the syntax errors and other type errors. I reported before that when I run the app at this time while it contains a bug, the run fails but the error list is empty! But all of this doesn't prevent the editor from formatting the code. Hence, if in this state I deleted a quote, and the code structure messed up so that literals become syntax tokens, no syntax errors are listed yet, and the editor will re format all the text (because it seems error-free), converting the case of what seems to be keywords!
Conclusion:
There is nothing wrong in the editor, but the component that checks for syntax errors (the compiler, the debugger, or what ever) is slow. This is not due to my old PC. I have the same issue on 2.3 GH / 4 core processor with 8 GB of RAM which should meet the requirements of VS.NET 2019.

@CyrusNajmabadi
Copy link
Member

I tried, but can't reproduce this issue in a test sample.

Without a repro, there's no real way to make a code change here as tehre's no way to actually validate any change would solve this issue.

Sometimes when I open the project with number of code files opened, VB compiler takes too long time to check the syntax errors and other type errors. I reported before that when I run the app at this time while it contains a bug, the run fails but the error list is empty!

That sounds like a different bug. Feel free to either file it, or add whatever information is necessary to the bug you filed to help get it resolved.

There is nothing wrong in the editor, but the component that checks for syntax errors (the compiler, the debugger, or what ever) is slow.

@VBAndCs You have been told several times how you can report issues with performance to the Roslyn team. Please follow those instructions.

Again, just repeating this info here doesn't do anything. Please actually provide the necessary information that has been asked for so that issues can be driven to resolution. It's quite possible you're running into perf bugs that no one else has seen. As such, the only way to fix them is to get you to actually submit real reports containing the necessary data.

@vatsalyaagrawal vatsalyaagrawal added Area-IDE Bug IDE-Formatter Code formatter and/or smart indent Need Design Review The end user experience design needs to be reviewed and approved. labels Apr 18, 2019
@vatsalyaagrawal vatsalyaagrawal added this to the Backlog milestone Apr 18, 2019
@VBAndCs
Copy link
Author

VBAndCs commented Apr 18, 2019

provide the necessary information that has been asked for

I have done that here:
https://developercommunity.visualstudio.com/content/problem/516485/vs-2019-editor-always-encounters-an-unrecoverable.html

@CyrusNajmabadi
Copy link
Member

@VBAndCs That doesn't contain the information being asked for. That's an issue where you ran into an unrecoverable error. If you're reporting slowness (as per (the compiler, the debugger, or what ever) is slow.), then please follow the instructions here: https://github.com/dotnet/roslyn/wiki/Reporting-Visual-Studio-crashes-and-performance-issues

@VBAndCs
Copy link
Author

VBAndCs commented Apr 20, 2019

I faced a similar issue just now:
I use this string in a test method1:

Dim z =
"@if (Model.Count == 0)
{
  @if (Model.Test)
  {
    <p>Test</p>
  }
  else
  {
    <p>Not Test</p>
  }
}
else
{
  <h1>Show Items</h1>
  @foreach (var item in Model)
  {
    @if (item.Id % 2 == 0)
    {
      <p class=" + Qt + "EvenItems" + Qt + ">item.Name</p>
    }
    else
    {
      <p class=" + Qt + "OddItems" + Qt + ">item.Name</p>
    }
  }
  <p>Done</p>
}"

I added a new test method2 in the class, and ran the tests, but the test method 2 failed. When I checked it, I found that this part of the string literal has changed by adding a space before EvenItems
<p class=" + Qt + " EvenItems" + Qt + ">item.Name</p>
and the same for:
<p class=" + Qt + " OddItems" + Qt + ">item.Name</p>

In another test method I hade this:

            z =
"@for (var i = 0; i < Model.Count; i++)
{
  <p>@i</p>
}"

The editor erased the space before Model so it became '<Model.Count`

I have 4 failed methods for the same reason: adding or deleting a space from a literal!
This time I can confirm that nothing is slow. I was working on this project for more than 5 hours, and VS was stable!
I don't know when the editor decide to format literals and why! This is frastrating!

@VBAndCs
Copy link
Author

VBAndCs commented Apr 20, 2019

@paul1956
Changing string format will not solve the problem as long as the editor is looking for the matching qoute across all the file, so that writing any starting quote in any part of the file can mess up all next code lines, regardless of how string literals are formated.

@VBAndCs
Copy link
Author

VBAndCs commented Apr 20, 2019

@CyrusNajmabadi
This issue is about a repeatedly random error, so, I can't tell you how to reproduce it. You have the code that have the issue, and you know how the editor works, so you can try to figure out how this issue happens. I don't think it is a good thing to close this issue. I expect I will see more of this soon, and I may notice the trigger. Also, other can have this issue, while moving into the newly porn VS.NET 2019. Closing is not the best action to do right now.
Thanks.

@CyrusNajmabadi
Copy link
Member

o that writing any starting quote in any part of the file can mess up all next code lines, regardless of how string literals are formated.

@VBAndCs I cannot repro this. what is your repro that shows that you can "mess up all next lines" by doing this? For all i know, this is due to some plugin or something else you're doing.

@CyrusNajmabadi
Copy link
Member

This issue is about a repeatedly random error, so, I can't tell you how to reproduce it.

So, how do you know it's roslyn that's doing this?

You have the code that have the issue, and you know how the editor works, so you can try to figure out how this issue happens.

I have tried. I can't figure anything out that would cause this. Everything i try locally (including deleting random quotes, or adding random quotes, or change code and editing around things), doens't repro any problems.

Also, other can have this issue, while moving into the newly porn VS.NET 2019

Nothing changed here. VB added this back in VS2015.

I don't think it is a good thing to close this issue.

  1. you haven't identified that roslyn is actually the problem here.
  2. you can't reproduce this issue yourself.
  3. you haven't provided a way for others to repro this.

What do you think can be done with the issue in this state?

@VBAndCs
Copy link
Author

VBAndCs commented Apr 21, 2019

@CyrusNajmabadi
The last issue is not about quotes only, as there is an additional literal: the xml literal. Adding or removing spaces around <' and '\> in string literals indicates the editor tries to format it as xml literals! My code has two kinds of literals, and each of them can be embedded in the other for the sake of parsing, code generation, and testing! Add to this they both can contain VB keywords, which I defined as xml tags and which are common with c# like if and double which appear in string literal! I don't think VB parser/Editor was tested for this unusual mix before!
So, you can try many different combination of these components. I advice you try adding a new sub in the middle of the file, and make a copy on of the code of others sub by hand as if you writing a new code. the problem can happing while writing any of these chars: ", >, <, /> </ writing in both xml literals and string literal. and try to encode <if> .. </if> and "@if () {}" . This is all what I am doing.
I will continue working and observing. I am not pushing you to do any thing, so don't feel obligated to provide s solution now. And sorry if I am not aware of which component can cause this. But if you inform other teams to investigate if you think it belongs to them. I also referred to this in one issue of VB.NET repo. If this happened a third time I will report if from VS.
Thank.

@CyrusNajmabadi
Copy link
Member

So, you can try many different combination of these components.

I have tried this. I cannot repro what you are saying.

I advice you try adding a new sub in the middle of the file, and make a copy on of the code of others sub by hand as if you writing a new code. the problem can happing while writing any of these chars: ", >, <, /> </ writing in both xml literals and string literal. and try to encode .. and "@if () {}" . This is all what I am doing.

I have tried this. I cannot repro what you are seeing.

I don't think VB parser/Editor was tested for this unusual mix before!

It definitely was. We've written tons of tests for both C# and VB using the VB editor. This includes lots of combnations of strings/xml-literals/etc. inside pieces of VB code. Many many thousands of tests have been written this way.

@VBAndCs
Copy link
Author

VBAndCs commented Apr 21, 2019

I caught one:

  1. Paste this sub:
    Sub TestElseIfs2()
        Dim x =
                <zml xmlns:z="zml">
                    <p>test</p>
                </zml>

        Dim y = x.ParseZml().ToString()
        Dim z =
"@if (grade < 30)
{
  <p>Very weak</p>
}
else
{
  <p>Excellent</p>
}"

        Assert.AreEqual(y, z)

    End Sub
  1. change </zml> to <p></p> Note that the editor with show an eeror in the string literal under < 30.

  2. Now, any action will reformat the string literal. Put the caret between the <p></p> and press enter. This will reformat the string literals not only in the current sub, but until the end of the file!

@CyrusNajmabadi
Copy link
Member

Now, any action will reformat the string literal.

This is not true. I was able to replace </zml> with

and then complete the` thus ending the xml snippet.

If you don't actually end the snippet then of course VB will think the remainder is part of the XML. That's how xml works after all :)

Put the caret between the

and press enter. This will reformat the string literals not only in the current sub, but until the end of the file!

I would recommend finishing your incomplete constructs.

Note: if you don't want automatic pretty-listing of code, i already showed how you could disable that here: #35028 (comment)

It seems you did not listen to that advice. If you have pretty listing enabled, then the LS will pretty list your code. If you don't want this happen automatically, then disable the feature. Then you can hit enter between <p></p> and nothing else will pretty list.

@VBAndCs
Copy link
Author

VBAndCs commented Apr 21, 2019

This is a bad design for xml literals. This issue can always happen when editing an xml literal such as pasting a part of a literal then starting to complete it, or mistakenly selecting the end tag with a previous part and deleting... etc.
Disabling the editor formatting is not a solution or you can say it is a zero solution for equations as in mathematics.
As I discussed above about the innocent string literals, if the editor found a broken xml literal it must stop formatting it until it is fixed. The editor can have a sign that what it thinks as an xml literal is actually containing a full vb.net lines of code, at least one method declaration statement. And as I said before, the editor can even copy the last correct syntax tree as a reference so a single edit can not compromise a whole code file
In fact, I encountered many issues with xml literals in last month, and reported them in vb repo. On top of that, the disability to use some & symbol folod with copy and some other famous shortcuts, the dysfunctional xsd schemas, and not preserving white spaces in xml literals that contains text nodes, which can be solved by passing an optional param to the XElement.Parse method when vb/Roslyn parses the literal.
Xml literals need more inhancements for sure, and I suggested more of them:
dotnet/vblang#397

@CyrusNajmabadi
Copy link
Member

Disabling the editor formatting is not a solution or you can say it is a zero solution for equations as in mathematics.

Why is it not a solution?

And as I said before, the editor can even copy the last correct syntax tree as a reference so a single edit can not compromise a whole code file

You can def contribute a fix here if you want :-)

@CyrusNajmabadi
Copy link
Member

The editor can have a sign that what it thinks as an xml literal is actually containing a full vb.net lines of code

I think that's what it does. The problem is that you have contents in your xml that actually look like code.

@vatsalyaagrawal vatsalyaagrawal removed the Need Design Review The end user experience design needs to be reviewed and approved. label Oct 31, 2019
@vatsalyaagrawal
Copy link
Contributor

vatsalyaagrawal commented Oct 31, 2019

Design review conclusion:

  • Our recommendation is to NOT run the line commit if the file has any unterminated string, since it is not clear which string is unterminated and the remaining code could be incorrectly altered. When line commit is skipped due to unterminated strings, the dirty region of the file should not be updated so the next line commit operation will reconsider them.
  • Follow-up: The same logic should apply to unterminated XML literals. Note that for XML literals, it may be possible to determine the specific element(s) which are not closed, and thus limit the locations where line commit is disabled.

@sharwell sharwell added the help wanted The issue is "up for grabs" - add a comment if you are interested in working on it label Oct 31, 2019
@VBAndCs
Copy link
Author

VBAndCs commented Nov 1, 2019

@vatsalyaagrawal
Thanks for your efforts.
Please note that the problem is not related to string literals (I tested that as described in the discussions above) but related to XML literals that doesn't have a proper closing tag, So, your conclusion should apply to dirty areas in XML literals.

@sharwell
Copy link
Member

sharwell commented Nov 1, 2019

@VBAndCs Thanks for the clarification. Yes, the same conclusion would apply to XML literals.

@VBAndCs
Copy link
Author

VBAndCs commented Jul 29, 2021

This issue state is Complete in IDE: Design review since 31 Oct 2019, almost two years ago! I still face issues because of this bug, the most recent one is a few minutes ago! I work a lot with parsers and currently I am working on Small Visual Basic project, where the parser deals with vb keywords, so, any change in casing can corrupt the parser. Luckily I retried the source back from my repository on github.
Please fix this bug.

@CyrusNajmabadi
Copy link
Member

Closing out as wr are not planning on investing in this area.

@CyrusNajmabadi CyrusNajmabadi closed this as not planned Won't fix, can't repro, duplicate, stale Nov 29, 2024
@github-project-automation github-project-automation bot moved this from InQueue to Completed in Small Fixes Nov 29, 2024
@dotnet dotnet locked and limited conversation to collaborators Nov 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Area-IDE Bug help wanted The issue is "up for grabs" - add a comment if you are interested in working on it IDE-Formatter Code formatter and/or smart indent
Projects
Status: Complete
Status: Completed
Development

No branches or pull requests

6 participants