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.08k stars 4.04k forks source link

InvalidOperationException from CSharpInitializeMemberFromPrimaryConstructorParameterCodeRefactoringProvider #69481

Open tmat opened 1 year ago

tmat commented 1 year ago
System.InvalidOperationException : Operation is not valid due to the current state of the object.
   at Microsoft.CodeAnalysis.CSharp.Extensions.SemanticModelExtensions.GetRequiredDeclaredSymbol(SemanticModel semanticModel,ParameterSyntax syntax,CancellationToken cancellationToken)
   at async Microsoft.CodeAnalysis.CSharp.InitializeParameter.CSharpInitializeMemberFromPrimaryConstructorParameterCodeRefactoringProvider.ComputeRefactoringsAsync(<Unknown Parameters>)
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at async Microsoft.CodeAnalysis.CodeRefactorings.CodeRefactoringService.GetRefactoringFromProviderAsync(<Unknown Parameters>)

Editing constructor in this code:

// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Linq;
using System.Reflection.Metadata;
using System.Runtime.InteropServices;
using System.Threading;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.CSharp.Test.Utilities;
using Microsoft.CodeAnalysis.Emit;
using Microsoft.CodeAnalysis.Test.Utilities;
using Roslyn.Test.Utilities;
using Xunit;

using Public = Microsoft.CodeAnalysis.CSharp.Symbols.PublicModel;

namespace Microsoft.CodeAnalysis.CSharp.EditAndContinue.UnitTests
{
    internal sealed class EditAndContinueTest(
        CSharpCompilationOptions? options = null,
        CSharpParseOptions? parseOptions = null,
        TargetFramework targetFramework = TargetFramework.Standard,
        Verification? verification = null)
        : EditAndContinueTest<CSharpCompilationOptions, CSharpParseOptions, CSharpCompilation>(
            options ?? EditAndContinueTestBase.ComSafeDebugDll,
            parseOptions ?? TestOptions.Regular.WithNoRefSafetyRulesAttribute(),
            targetFramework,
            verification)
    {
        internal EditAndContinueTest AddBaseline(string source, Action<GenerationVerifier> validator)
            => AddBaseline(EditAndContinueTestBase.MarkedSource(source, options: parseOptions), validator);

        protected override Compilation CreateCompilation(SyntaxTree tree, CSharpCompilationOptions options)
            => CSharpTestBase.CreateCompilation(tree, options: options, targetFramework: targetFramework);
    }

    internal abstract partial class EditAndContinueTest(Verification? verification = null) : IDisposable
        where TCompilationOptions : CompilationOptions
        where TParseOptions : ParseOptions
    {
        private readonly Verification _verification = verification ?? Verification.Passes;
        private readonly List<IDisposable> _disposables = new();
        private readonly List<GenerationInfo> _generations = new();
        private readonly List<SourceWithMarkedNodes> _sources = new();

        private bool _hasVerified;

        protected abstract Compilation CreateCompilation(SyntaxTree tree);

        internal EditAndContinueTest AddBaseline(SourceWithMarkedNodes source, Action<GenerationVerifier> validator)
        {
            _hasVerified = false;

            Assert.Empty(_generations);

            var compilation = CreateCompilation(source.Tree);

            var verifier = new CompilationVerifier(compilation);

            verifier.Emit(
                expectedOutput: null,
                trimOutput: false,
                expectedReturnCode: null,
                args: null,
                manifestResources: null,
                emitOptions: EmitOptions.Default,
                peVerify: _verification,
                expectedSignatures: null);

            var md = ModuleMetadata.CreateFromImage(verifier.EmittedAssemblyData);
            _disposables.Add(md);

            var baseline = EmitBaseline.CreateInitialBaseline(md, verifier.CreateSymReader().GetEncMethodDebugInfo);

            _generations.Add(new GenerationInfo(compilation, md.MetadataReader, diff: null, verifier, baseline, validator));
            _sources.Add(source);

            return this;
        }

        internal EditAndContinueTest AddGeneration(string source, SemanticEditDescription[] edits, Action<GenerationVerifier> validator)
            => AddGeneration(EditAndContinueTestBase.MarkedSource(source), edits, validator);

        internal EditAndContinueTest AddGeneration(SourceWithMarkedNodes source, SemanticEditDescription[] edits, Action<GenerationVerifier> validator)
        {
            _hasVerified = false;

            Assert.NotEmpty(_generations);
            Assert.NotEmpty(_sources);

            var previousGeneration = _generations[^1];
            var previousSource = _sources[^1];

            Assert.Equal(previousSource.MarkedSpans.IsEmpty, source.MarkedSpans.IsEmpty);

            var compilation = previousGeneration.Compilation.WithSource(source.Tree);

            var unmappedNodes = new List<SyntaxNode>();

            var semanticEdits = GetSemanticEdits(edits, previousGeneration.Compilation, previousSource, compilation, source, unmappedNodes);

            CompilationDifference diff = compilation.EmitDifference(previousGeneration.Baseline, semanticEdits);

            Assert.Empty(diff.EmitResult.Diagnostics.Where(d => d.Severity == DiagnosticSeverity.Error));

            // EncVariableSlotAllocator attempted to map from current source to the previous one,
            // but the mapping failed for these nodes. Mark the nodes in sources with node markers <N:x>...</N:x>.
            Assert.Empty(unmappedNodes);

            var md = diff.GetMetadata();
            _disposables.Add(md);

            _generations.Add(new GenerationInfo(compilation, md.Reader, diff, compilationVerifier: null, diff.NextGeneration, validator));
            _sources.Add(source);

            return this;
        }

        internal EditAndContinueTest Verify()
        {
            _hasVerified = true;

            Assert.NotEmpty(_generations);

            var readers = new List<MetadataReader>();
            int index = 0;
            foreach (var generation in _generations)
            {
                if (readers.Count > 0)
                {
                    EncValidation.VerifyModuleMvid(index, readers[^1], generation.MetadataReader);
                }

                readers.Add(generation.MetadataReader);
                var verifier = new GenerationVerifier(index, generation, readers);
                generation.Verifier(verifier);

                index++;
            }

            return this;
        }

        private ImmutableArray<SemanticEdit> GetSemanticEdits(
            SemanticEditDescription[] edits,
            Compilation oldCompilation,
            SourceWithMarkedNodes oldSource,
            Compilation newCompilation,
            SourceWithMarkedNodes newSource,
            List<SyntaxNode> unmappedNodes)
        {
            var syntaxMapFromMarkers = oldSource.MarkedSpans.IsEmpty ? null : SourceWithMarkedNodes.GetSyntaxMap(oldSource, newSource, unmappedNodes);

            return ImmutableArray.CreateRange(edits.Select(e =>
            {
                var oldSymbol = e.Kind is SemanticEditKind.Update or SemanticEditKind.Delete ? e.SymbolProvider(oldCompilation) : null;

                // for delete the new symbol is the new containing type
                var newSymbol = e.NewSymbolProvider(newCompilation);

                Func<SyntaxNode, SyntaxNode?>? syntaxMap;
                if (e.PreserveLocalVariables)
                {
                    Assert.Equal(SemanticEditKind.Update, e.Kind);
                    Debug.Assert(oldSymbol != null);
                    Debug.Assert(newSymbol != null);

                    syntaxMap = syntaxMapFromMarkers ?? EditAndContinueTestBase.GetEquivalentNodesMap(
                        ((Public.MethodSymbol)newSymbol).GetSymbol<MethodSymbol>(), ((Public.MethodSymbol)oldSymbol).GetSymbol<MethodSymbol>());
                }
                else
                {
                    syntaxMap = null;
                }

                return new SemanticEdit(e.Kind, oldSymbol, newSymbol, syntaxMap, e.PreserveLocalVariables);
            }));
        }

        public void Dispose()
        {
            // If the test has thrown an exception, or the test host has crashed, we don't want to assert here
            // or we'll hide it, so we need to do this dodgy looking thing.
            var isInException = Marshal.GetExceptionPointers() != IntPtr.Zero;

            Assert.True(isInException || _hasVerified, "No Verify call since the last AddGeneration call.");
            foreach (var disposable in _disposables)
            {
                disposable.Dispose();
            }
        }
    }
}
CyrusNajmabadi commented 1 year ago

This is fairly crazy, as teh only place we call this is on a ParameterDeclarationSyntax. But we're somehow getting null back.

CyrusNajmabadi commented 1 year ago

need @dotnet/roslyn-compiler to take a look. Specifically, this seems to break the invariant that all XXXDeclarationSyntax nodes hve a corresponding symbol associated with them.

Seeing if i can figure out which parameter this fails for.

CyrusNajmabadi commented 1 year ago

Not repro'ing anything on latest bits. Maybe an issue with an older sdk and primary constructors. Looking to see if we have any prism hits.

CyrusNajmabadi commented 1 year ago

Haven't heard about this from any other sources. Prism shows no hits for this as well. My guess is some bad combo of things that caused this to happen :)