dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.04k stars 4.03k forks source link

Invalid formatting of `#If` directive in VB #50117

Open AlekseyTs opened 3 years ago

AlekseyTs commented 3 years ago

Paste the following code in VB type.

        /// <summary>
        /// Generate a list containing the given field and all dependencies
        /// of that field that require evaluation. The list is ordered by
        /// dependencies, with fields with no dependencies first. Cycles are
        /// broken at the first field lexically in the cycle. If multiple threads
        /// call this method with the same field, the order of the fields
        /// returned should be the same, although some fields may be missing
        /// from the lists in some threads as other threads evaluate fields.
        /// </summary>
        internal static void OrderAllDependencies(
            this SourceFieldSymbolWithSyntaxReference field,
            ArrayBuilder<FieldInfo> order,
            bool earlyDecodingWellKnownAttributes)
        {
            Debug.Assert(order.Count == 0);

            var graph = PooledDictionary<SourceFieldSymbolWithSyntaxReference, Node<SourceFieldSymbolWithSyntaxReference>>.GetInstance();

            CreateGraph(graph, field, earlyDecodingWellKnownAttributes);

            Debug.Assert(graph.Count >= 1);
            CheckGraph(graph);

#if DEBUG
            var fields = ArrayBuilder<SourceFieldSymbolWithSyntaxReference>.GetInstance();
            fields.AddRange(graph.Keys);
#endif

            OrderGraph(graph, order);

#if DEBUG
            // Verify all entries in the graph are in the ordered list.
            var map = new HashSet<SourceFieldSymbolWithSyntaxReference>(order.Select(o => o.Field).Distinct());
            Debug.Assert(fields.All(f => map.Contains(f)));
            fields.Free();
#endif

            graph.Free();
        }

Observed:

        /// <summary>
        /// Generate a list containing the given field And all dependencies
        /// of that field that require evaluation. The list Is ordered by
        /// dependencies, with fields with no dependencies first. Cycles are
        /// broken at the first field lexically in the cycle. If multiple threads
        /// call this method with the same field, the order of the fields
        /// returned should be the same, although some fields may be missing
        /// from the lists in some threads as other threads evaluate fields.
        /// </summary>
        internal Static void OrderAllDependencies(
            this SourceFieldSymbolWithSyntaxReference field,
            ArrayBuilder<FieldInfo> order,
            bool earlyDecodingWellKnownAttributes)
        {
            Debug.Assert(order.Count == 0);

            var graph = PooledDictionary < SourceFieldSymbolWithSyntaxReference, Node<SourceFieldSymbolWithSyntaxReference>>.GetInstance();

            CreateGraph(graph, field, earlyDecodingWellKnownAttributes);

            Debug.Assert(graph.Count >= 1);
            CheckGraph(graph);

#If DEBUGThen
            var fields = ArrayBuilder < SourceFieldSymbolWithSyntaxReference > .GetInstance();
            fields.AddRange(graph.Keys);
#End If

            OrderGraph(graph, order);

#if DEBUGThen
            // Verify all entries in the graph are in the ordered list.
            var map = New HashSet < SourceFieldSymbolWithSyntaxReference > (order.Select(o >= o.Field).Distinct());
            Debug.Assert(fields.All(f => map.Contains(f)));
            fields.Free();
#End If

            graph.Free();
        }

Note, conditional compilation directive is changed to #if DEBUGThen. Expected #if DEBUG Then.

Also, if I position the cursor at the end of #if DEBUGThen and press enter, the result is the following:

#If DEBUGThenThen

#End If

The space is missing and #End If is added. This is unexpected, #End If is already present and shouldn't be added.

AlekseyTs commented 3 years ago

Don't have exact repro steps, but I've also seen Then added to condition of a regular IF statement without a space in between. Initially that was a C# code pasted into VB, lots of error around it, and, while I was editing code to fix them up, Then was added.

sharwell commented 3 years ago

I believe the proposed resolution is to not run the Code Cleanup features of line commit if the file contains syntax errors.

AlekseyTs commented 3 years ago

I see nothing wrong in addition of Then in these scenarios, but it should be added with a space before it.