diff --git a/src/Compilers/CSharp/Portable/Binder/SwitchBinder.cs b/src/Compilers/CSharp/Portable/Binder/SwitchBinder.cs index 182f4c37edffaa6013927ba368e12330211a82d8..d6643115641efa10bb5ca04a08fb9eabc91574f6 100644 --- a/src/Compilers/CSharp/Portable/Binder/SwitchBinder.cs +++ b/src/Compilers/CSharp/Portable/Binder/SwitchBinder.cs @@ -131,14 +131,14 @@ protected override ImmutableArray BuildLabels() if (!IsPatternSwitch(_switchSyntax)) { - foreach (var section in _switchSyntax.Sections) - { - // add switch case/default labels - BuildSwitchLabels(section.Labels, ref labels); + foreach (var section in _switchSyntax.Sections) + { + // add switch case/default labels + BuildSwitchLabels(section.Labels, ref labels); - // add regular labels from the statements in the switch section - base.BuildLabels(section.Statements, ref labels); - } + // add regular labels from the statements in the switch section + base.BuildLabels(section.Statements, ref labels); + } } return (labels != null) ? labels.ToImmutableAndFree() : ImmutableArray.Empty; @@ -155,22 +155,22 @@ private void BuildSwitchLabels(SyntaxList labelsSyntax, ref A switch (labelSyntax.Kind()) { case SyntaxKind.CaseSwitchLabel: - // Bind the switch expression and the switch case label expression, but do not report any diagnostics here. - // Diagnostics will be reported during binding. - var caseLabel = (CaseSwitchLabelSyntax)labelSyntax; - Debug.Assert(caseLabel.Value != null); - DiagnosticBag tempDiagnosticBag = DiagnosticBag.GetInstance(); + // Bind the switch expression and the switch case label expression, but do not report any diagnostics here. + // Diagnostics will be reported during binding. + var caseLabel = (CaseSwitchLabelSyntax)labelSyntax; + Debug.Assert(caseLabel.Value != null); + DiagnosticBag tempDiagnosticBag = DiagnosticBag.GetInstance(); - var boundLabelExpression = BindValue(caseLabel.Value, tempDiagnosticBag, BindValueKind.RValue); + var boundLabelExpression = BindValue(caseLabel.Value, tempDiagnosticBag, BindValueKind.RValue); - if ((object)switchGoverningType == null) - { - switchGoverningType = this.BindSwitchExpression(_switchSyntax.Expression, tempDiagnosticBag).Type; - } + if ((object)switchGoverningType == null) + { + switchGoverningType = this.BindSwitchExpression(_switchSyntax.Expression, tempDiagnosticBag).Type; + } - boundLabelExpression = ConvertCaseExpression(switchGoverningType, labelSyntax, boundLabelExpression, ref boundLabelConstantOpt, tempDiagnosticBag); + boundLabelExpression = ConvertCaseExpression(switchGoverningType, labelSyntax, boundLabelExpression, ref boundLabelConstantOpt, tempDiagnosticBag); - tempDiagnosticBag.Free(); + tempDiagnosticBag.Free(); break; case SyntaxKind.DefaultSwitchLabel: @@ -346,7 +346,7 @@ internal override BoundStatement BindSwitchExpressionAndSections(SwitchStatement // Bind switch section ImmutableArray boundSwitchSections = BindSwitchSections(node.Sections, originalBinder, diagnostics); - return new BoundSwitchStatement(node, boundSwitchExpression, constantTargetOpt, Locals, LocalFunctions, boundSwitchSections, this.BreakLabel, null); + return new BoundSwitchStatement(node, null, boundSwitchExpression, constantTargetOpt, Locals, LocalFunctions, boundSwitchSections, this.BreakLabel, null); } // Bind the switch expression and set the switch governing type @@ -519,29 +519,29 @@ private BoundSwitchLabel BindSwitchSectionLabel(SwitchLabelSyntax node, Diagnost switch (node.Kind()) { case SyntaxKind.CaseSwitchLabel: - var caseLabelSyntax = (CaseSwitchLabelSyntax)node; - // Bind the label case expression - boundLabelExpressionOpt = BindValue(caseLabelSyntax.Value, diagnostics, BindValueKind.RValue); + var caseLabelSyntax = (CaseSwitchLabelSyntax)node; + // Bind the label case expression + boundLabelExpressionOpt = BindValue(caseLabelSyntax.Value, diagnostics, BindValueKind.RValue); - boundLabelExpressionOpt = ConvertCaseExpression(switchGoverningType, caseLabelSyntax, boundLabelExpressionOpt, ref labelExpressionConstant, diagnostics); + boundLabelExpressionOpt = ConvertCaseExpression(switchGoverningType, caseLabelSyntax, boundLabelExpressionOpt, ref labelExpressionConstant, diagnostics); - // Check for bind errors - hasErrors = hasErrors || boundLabelExpressionOpt.HasAnyErrors; + // Check for bind errors + hasErrors = hasErrors || boundLabelExpressionOpt.HasAnyErrors; - // SPEC: The constant expression of each case label must denote a value that - // SPEC: is implicitly convertible (§6.1) to the governing type of the switch statement. + // SPEC: The constant expression of each case label must denote a value that + // SPEC: is implicitly convertible (§6.1) to the governing type of the switch statement. - // Prevent cascading diagnostics - if (!hasErrors && labelExpressionConstant == null) - { - diagnostics.Add(ErrorCode.ERR_ConstantExpected, caseLabelSyntax.Location); - hasErrors = true; - } + // Prevent cascading diagnostics + if (!hasErrors && labelExpressionConstant == null) + { + diagnostics.Add(ErrorCode.ERR_ConstantExpected, caseLabelSyntax.Location); + hasErrors = true; + } - // LabelSymbols for all the switch case labels are created by BuildLabels(). - // Fetch the matching switch case label symbols - matchedLabelSymbols = FindMatchingSwitchCaseLabels(labelExpressionConstant, caseLabelSyntax); + // LabelSymbols for all the switch case labels are created by BuildLabels(). + // Fetch the matching switch case label symbols + matchedLabelSymbols = FindMatchingSwitchCaseLabels(labelExpressionConstant, caseLabelSyntax); break; case SyntaxKind.CasePatternSwitchLabel: // pattern matching in case is not yet implemented. @@ -549,11 +549,11 @@ private BoundSwitchLabel BindSwitchSectionLabel(SwitchLabelSyntax node, Diagnost { Error(diagnostics, ErrorCode.ERR_FeatureIsUnimplemented, node, MessageID.IDS_FeaturePatternMatching.Localize()); hasErrors = true; - } + } matchedLabelSymbols = new List(); break; case SyntaxKind.DefaultSwitchLabel: - matchedLabelSymbols = GetDefaultLabels(); + matchedLabelSymbols = GetDefaultLabels(); break; default: throw ExceptionUtilities.Unreachable; diff --git a/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml b/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml index a41aeaf75d73028191f745c9a9279857a50e6074..26f118790561b46476c7152079a0557f3ae778ec 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml +++ b/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml @@ -623,8 +623,8 @@ - - @@ -682,6 +682,7 @@ + @@ -1400,7 +1401,7 @@ - + diff --git a/src/Compilers/CSharp/Portable/CodeGen/EmitStatement.cs b/src/Compilers/CSharp/Portable/CodeGen/EmitStatement.cs index 9f93d7c6595f08243f60a55fa7b55669f1de4703..c62d1f537669ee128c7be6d74a40c5324eba343a 100644 --- a/src/Compilers/CSharp/Portable/CodeGen/EmitStatement.cs +++ b/src/Compilers/CSharp/Portable/CodeGen/EmitStatement.cs @@ -5,12 +5,9 @@ using System.Collections.Immutable; using System.Diagnostics; using System.Linq; -using System.Reflection.Metadata; using Microsoft.CodeAnalysis.CodeGen; -using Microsoft.CodeAnalysis.Collections; using Microsoft.CodeAnalysis.CSharp.Symbols; using Microsoft.CodeAnalysis.CSharp.Syntax; -using Microsoft.CodeAnalysis.Symbols; using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; @@ -1029,6 +1026,12 @@ private void EmitCatchBlock(BoundCatchBlock catchBlock) private void EmitSwitchStatement(BoundSwitchStatement switchStatement) { + var preambleOpt = switchStatement.LoweredPreambleOpt; + if (preambleOpt != null) + { + EmitStatement(preambleOpt); + } + // Switch expression must have a valid switch governing type Debug.Assert((object)switchStatement.Expression.Type != null); Debug.Assert(switchStatement.Expression.Type.IsValidSwitchGoverningType()); @@ -1570,11 +1573,12 @@ public override BoundNode VisitLabelStatement(BoundLabelStatement node) public override BoundNode VisitSwitchStatement(BoundSwitchStatement node) { var breakLabelClone = GetLabelClone(node.BreakLabel); + var preambleOpt = (BoundStatement)this.Visit(node.LoweredPreambleOpt); // expressions do not contain labels or branches BoundExpression boundExpression = node.Expression; ImmutableArray switchSections = (ImmutableArray)this.VisitList(node.SwitchSections); - return node.Update(boundExpression, node.ConstantTargetOpt, node.InnerLocals, node.InnerLocalFunctions, switchSections, breakLabelClone, node.StringEquality); + return node.Update(preambleOpt, boundExpression, node.ConstantTargetOpt, node.InnerLocals, node.InnerLocalFunctions, switchSections, breakLabelClone, node.StringEquality); } public override BoundNode VisitSwitchLabel(BoundSwitchLabel node) diff --git a/src/Compilers/CSharp/Portable/CodeGen/Optimizer.cs b/src/Compilers/CSharp/Portable/CodeGen/Optimizer.cs index 3a96ee468e41a86b8d6bd15abc74f8053128458c..425a57207923df23d95c244055ee87d6935936e5 100644 --- a/src/Compilers/CSharp/Portable/CodeGen/Optimizer.cs +++ b/src/Compilers/CSharp/Portable/CodeGen/Optimizer.cs @@ -273,7 +273,7 @@ public void Free() { _localDefs.Free(); _localDefs = null; - } + } _pool?.Free(this); } @@ -378,7 +378,7 @@ internal enum ExprContext Box } - // Analyses the tree trying to figure which locals may live on stack. + // Analyzes the tree trying to figure which locals may live on stack. // It is a fairly delicate process and must be very familiar with how CodeGen works. // It is essentially a part of CodeGen. // @@ -1413,6 +1413,9 @@ public override BoundNode VisitUnaryOperator(BoundUnaryOperator node) public override BoundNode VisitSwitchStatement(BoundSwitchStatement node) { Debug.Assert(EvalStackIsEmpty()); + + var preambleOpt = (BoundStatement)this.Visit(node.LoweredPreambleOpt); + DeclareLocals(node.InnerLocals, 0); // switch needs a byval local or a parameter as a key. @@ -1445,7 +1448,7 @@ public override BoundNode VisitSwitchStatement(BoundSwitchStatement node) this.RecordLabel(breakLabel); } - var result = node.Update(boundExpression, node.ConstantTargetOpt, node.InnerLocals, node.InnerLocalFunctions, switchSections, breakLabel, node.StringEquality); + var result = node.Update(preambleOpt, boundExpression, node.ConstantTargetOpt, node.InnerLocals, node.InnerLocalFunctions, switchSections, breakLabel, node.StringEquality); // implicit control flow EnsureOnlyEvalStack(); diff --git a/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs b/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs index 5646308611aaaf00babd67bd50e26ce9e33f0589..93a0b82da0813b1d81b9a595f6b98aa586b8b3bf 100644 --- a/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs +++ b/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs @@ -325,7 +325,7 @@ public override INamedTypeSymbol CreateErrorTypeSymbol(INamespaceOrTypeSymbol co _syntaxAndDeclarations = syntaxAndDeclarations; Debug.Assert((object)_lazyAssemblySymbol == null); - if (EventQueue != null) EventQueue.Enqueue(new CompilationStartedEvent(this)); + if (EventQueue != null) EventQueue.TryEnqueue(new CompilationStartedEvent(this)); } internal override void ValidateDebugEntryPoint(IMethodSymbol debugEntryPoint, DiagnosticBag diagnostics) @@ -1716,13 +1716,15 @@ private void CompleteTree(SyntaxTree tree) if (completedCompilationUnit) { - EventQueue.Enqueue(new CompilationUnitCompletedEvent(this, tree)); + EventQueue.TryEnqueue(new CompilationUnitCompletedEvent(this, tree)); } if (completedCompilation) { - EventQueue.Enqueue(new CompilationCompletedEvent(this)); - EventQueue.Complete(); // signal the end of compilation events + // signal the end of compilation events + EventQueue.TryEnqueue(new CompilationCompletedEvent(this)); + EventQueue.PromiseNotToEnqueue(); + EventQueue.TryComplete(); } } @@ -2886,7 +2888,7 @@ internal override AnalyzerDriver AnalyzerForLanguage(ImmutableArray diff --git a/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs b/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs index 91b7f73ab1414c4fbc1d28833fe446835ff3b564..3eb23e1ba9abb048dc5228f4689d81a74a7a20a6 100644 --- a/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs +++ b/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs @@ -958,7 +958,7 @@ public override object VisitField(FieldSymbol symbol, TypeCompilationState argum }); MethodSymbol symbolToProduce = methodSymbol.PartialDefinitionPart ?? methodSymbol; - _compilation.EventQueue.Enqueue(new SymbolDeclaredCompilationEvent(_compilation, symbolToProduce, lazySemanticModel)); + _compilation.EventQueue.TryEnqueue(new SymbolDeclaredCompilationEvent(_compilation, symbolToProduce, lazySemanticModel)); } // Don't lower if we're not emitting or if there were errors. diff --git a/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncExceptionHandlerRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncExceptionHandlerRewriter.cs index 995733ad61adc24a93fb7001ffaa855edc4a934c..5b3a44b69f11ecf4c6a3e5ca74abcfba112cbd9c 100644 --- a/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncExceptionHandlerRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncExceptionHandlerRewriter.cs @@ -708,7 +708,7 @@ private void PopFrame() } /// - /// Analyses method body for try blocks with awaits in finally blocks + /// Analyzes method body for try blocks with awaits in finally blocks /// Also collects labels that such blocks contain. /// private sealed class AwaitInFinallyAnalysis : LabelCollector diff --git a/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AwaitExpressionSpiller.cs b/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AwaitExpressionSpiller.cs index d67b00bb56dfa0217ab8e30ccffe96da1481905c..475cb76ca9aeedfc91419e05ad2cd8a04877cfde 100644 --- a/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AwaitExpressionSpiller.cs +++ b/src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AwaitExpressionSpiller.cs @@ -518,9 +518,10 @@ public override BoundNode VisitSwitchStatement(BoundSwitchStatement node) EnterStatement(node); BoundSpillSequenceBuilder builder = null; + var preambleOpt = (BoundStatement)this.Visit(node.LoweredPreambleOpt); var boundExpression = VisitExpression(ref builder, node.Expression); var switchSections = this.VisitList(node.SwitchSections); - return UpdateStatement(builder, node.Update(boundExpression, node.ConstantTargetOpt, node.InnerLocals, node.InnerLocalFunctions, switchSections, node.BreakLabel, node.StringEquality), substituteTemps: true); + return UpdateStatement(builder, node.Update(preambleOpt, boundExpression, node.ConstantTargetOpt, node.InnerLocals, node.InnerLocalFunctions, switchSections, node.BreakLabel, node.StringEquality), substituteTemps: true); } public override BoundNode VisitThrowStatement(BoundThrowStatement node) diff --git a/src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorMethodToStateMachineRewriter.YieldsInTryAnalysis.cs b/src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorMethodToStateMachineRewriter.YieldsInTryAnalysis.cs index 2dc988c7cd992b6183106b95268be4aa8a2e1ecc..07cfdee9644ad1abb5c4cc3bbdaaa3b82f97bd94 100644 --- a/src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorMethodToStateMachineRewriter.YieldsInTryAnalysis.cs +++ b/src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorMethodToStateMachineRewriter.YieldsInTryAnalysis.cs @@ -9,7 +9,7 @@ namespace Microsoft.CodeAnalysis.CSharp internal partial class IteratorMethodToStateMachineRewriter { /// - /// Analyses method body for yields in try blocks and labels that they contain. + /// Analyzes method body for yields in try blocks and labels that they contain. /// private sealed class YieldsInTryAnalysis : LabelCollector { @@ -110,7 +110,7 @@ public override BoundNode VisitExpressionStatement(BoundExpressionStatement node } /// - /// Analyses method body for labels. + /// Analyzes method body for labels. /// internal abstract class LabelCollector : BoundTreeWalkerWithStackGuardWithoutRecursionOnTheLeftOfBinaryOperator { diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_SwitchStatement.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_SwitchStatement.cs index f8004260fcd0e84637a2f3f4184b4ef3eae84f36..a137b72182987c0ea7c50135d473705745dba6c0 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_SwitchStatement.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_SwitchStatement.cs @@ -84,11 +84,12 @@ public override BoundNode VisitSwitchStatement(BoundSwitchStatement node) return rewrittenExpression.Type.IsNullableType() ? MakeSwitchStatementWithNullableExpression(syntax, rewrittenExpression, rewrittenSections, constantTargetOpt, locals, localFunctions, breakLabel, oldNode) : - MakeSwitchStatementWithNonNullableExpression(syntax, rewrittenExpression, rewrittenSections, constantTargetOpt, locals, localFunctions, breakLabel, oldNode); + MakeSwitchStatementWithNonNullableExpression(syntax, null, rewrittenExpression, rewrittenSections, constantTargetOpt, locals, localFunctions, breakLabel, oldNode); } private BoundStatement MakeSwitchStatementWithNonNullableExpression( CSharpSyntaxNode syntax, + BoundStatement preambleOpt, BoundExpression rewrittenExpression, ImmutableArray rewrittenSections, LabelSymbol constantTargetOpt, @@ -112,6 +113,7 @@ public override BoundNode VisitSwitchStatement(BoundSwitchStatement node) } return oldNode.Update( + loweredPreambleOpt: preambleOpt, expression: rewrittenExpression, constantTargetOpt: constantTargetOpt, innerLocals: locals, @@ -161,7 +163,6 @@ public override BoundNode VisitSwitchStatement(BoundSwitchStatement node) condition: MakeNullCheck(exprSyntax, rewrittenExpression, BinaryOperatorKind.NullableNullEqual), jumpIfTrue: true, label: GetNullValueTargetSwitchLabel(rewrittenSections, breakLabel)); - statementBuilder.Add(condGotoNullValueTargetLabel); // Rewrite the switch statement using nullable expression's underlying value as the switch expression. @@ -171,8 +172,16 @@ public override BoundNode VisitSwitchStatement(BoundSwitchStatement node) rewrittenExpression = callGetValueOrDefault; // rewrite switch statement - BoundStatement rewrittenSwitchStatement = MakeSwitchStatementWithNonNullableExpression(syntax, - rewrittenExpression, rewrittenSections, constantTargetOpt, locals, localFunctions, breakLabel, oldNode); + BoundStatement rewrittenSwitchStatement = MakeSwitchStatementWithNonNullableExpression( + syntax, + condGotoNullValueTargetLabel, + rewrittenExpression, + rewrittenSections, + constantTargetOpt, + locals, + localFunctions, + breakLabel, + oldNode); statementBuilder.Add(rewrittenSwitchStatement); diff --git a/src/Compilers/CSharp/Portable/Lowering/MethodToClassRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/MethodToClassRewriter.cs index c6a17829963c2fcc7c682f57341710adbfa802c1..de4f4cfada4d137556ad01dc5d647763dab8a54f 100644 --- a/src/Compilers/CSharp/Portable/Lowering/MethodToClassRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/MethodToClassRewriter.cs @@ -147,10 +147,11 @@ public override BoundNode VisitSequence(BoundSequence node) public override BoundNode VisitSwitchStatement(BoundSwitchStatement node) { + var preambleOpt = (BoundStatement)this.Visit(node.LoweredPreambleOpt); var newInnerLocals = RewriteLocals(node.InnerLocals); BoundExpression boundExpression = (BoundExpression)this.Visit(node.Expression); ImmutableArray switchSections = (ImmutableArray)this.VisitList(node.SwitchSections); - return node.Update(boundExpression, node.ConstantTargetOpt, newInnerLocals, node.InnerLocalFunctions, switchSections, node.BreakLabel, node.StringEquality); + return node.Update(preambleOpt, boundExpression, node.ConstantTargetOpt, newInnerLocals, node.InnerLocalFunctions, switchSections, node.BreakLabel, node.StringEquality); } public override BoundNode VisitForStatement(BoundForStatement node) diff --git a/src/Compilers/CSharp/Portable/Lowering/SyntheticBoundNodeFactory.cs b/src/Compilers/CSharp/Portable/Lowering/SyntheticBoundNodeFactory.cs index 66d39bdc09b25431f385d00c7f6e00c86a200025..005c51535135f25157b7a7c00b2775df7143f31a 100644 --- a/src/Compilers/CSharp/Portable/Lowering/SyntheticBoundNodeFactory.cs +++ b/src/Compilers/CSharp/Portable/Lowering/SyntheticBoundNodeFactory.cs @@ -763,6 +763,7 @@ public BoundStatement Switch(BoundExpression ex, params BoundSwitchSection[] sec CheckSwitchSections(s); return new BoundSwitchStatement( Syntax, + null, ex, null, ImmutableArray.Empty, diff --git a/src/Compilers/CSharp/Portable/Symbols/SymbolDistinguisher.cs b/src/Compilers/CSharp/Portable/Symbols/SymbolDistinguisher.cs index 87999062ff4819e013f3d016d1dcbc8bbafcca71..bfb99f7869edd17baef3b08cc740a211c2470008 100644 --- a/src/Compilers/CSharp/Portable/Symbols/SymbolDistinguisher.cs +++ b/src/Compilers/CSharp/Portable/Symbols/SymbolDistinguisher.cs @@ -223,9 +223,13 @@ public override bool Equals(object obj) public override int GetHashCode() { - return Hash.Combine( - _distinguisher._compilation.GetHashCode(), - GetSymbol().GetHashCode()); + int result = GetSymbol().GetHashCode(); + var compilation = _distinguisher._compilation; + if (compilation != null) + { + result = Hash.Combine(result, compilation.GetHashCode()); + } + return result; } public override string ToString() diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenClosureLambdaTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenClosureLambdaTests.cs index b0a9c84ddabe50c8f2f2f1b5c8f895d8b885a92b..def8dd4cd9b3dd597cf3dccf3e27d3d4ed4db2e6 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenClosureLambdaTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenClosureLambdaTests.cs @@ -5224,5 +5224,73 @@ public object Foo() }"; CompileAndVerify(source, new[] { SystemCoreRef }); } + + [WorkItem(9131, "https://github.com/dotnet/roslyn/issues/9131")] + [Fact] + public void ClosureInSwitchStatementWithNullableExpression() + { + string source = +@"using System; +class C +{ + static void Main() + { + int? i = null; + switch (i) + { + default: + object o = null; + Func f = () => o; + Console.Write(""{0}"", f() == null); + break; + case 0: + o = 1; + break; + } + } +}"; + var compilation = CompileAndVerify(source, expectedOutput: @"True"); + compilation.VerifyIL("C.Main", +@"{ + // Code size 92 (0x5c) + .maxstack 3 + .locals init (int? V_0, //i + C.<>c__DisplayClass0_0 V_1, //CS$<>8__locals0 + int V_2, + System.Func V_3) //f + IL_0000: ldloca.s V_0 + IL_0002: initobj ""int?"" + IL_0008: newobj ""C.<>c__DisplayClass0_0..ctor()"" + IL_000d: stloc.1 + IL_000e: ldloca.s V_0 + IL_0010: call ""bool int?.HasValue.get"" + IL_0015: brfalse.s IL_0022 + IL_0017: ldloca.s V_0 + IL_0019: call ""int int?.GetValueOrDefault()"" + IL_001e: stloc.2 + IL_001f: ldloc.2 + IL_0020: brfalse.s IL_004f + IL_0022: ldloc.1 + IL_0023: ldnull + IL_0024: stfld ""object C.<>c__DisplayClass0_0.o"" + IL_0029: ldloc.1 + IL_002a: ldftn ""object C.<>c__DisplayClass0_0.
b__0()"" + IL_0030: newobj ""System.Func..ctor(object, System.IntPtr)"" + IL_0035: stloc.3 + IL_0036: ldstr ""{0}"" + IL_003b: ldloc.3 + IL_003c: callvirt ""object System.Func.Invoke()"" + IL_0041: ldnull + IL_0042: ceq + IL_0044: box ""bool"" + IL_0049: call ""void System.Console.Write(string, object)"" + IL_004e: ret + IL_004f: ldloc.1 + IL_0050: ldc.i4.1 + IL_0051: box ""int"" + IL_0056: stfld ""object C.<>c__DisplayClass0_0.o"" + IL_005b: ret +}"); + } } } diff --git a/src/Compilers/CSharp/Test/Semantic/Diagnostics/GetDiagnosticsTests.cs b/src/Compilers/CSharp/Test/Semantic/Diagnostics/GetDiagnosticsTests.cs index 1ae605eba5cfa63269884ddbd21f466366930974..2fad7970de519c6e38f6d61c9cc3c3e85b62a39e 100644 --- a/src/Compilers/CSharp/Test/Semantic/Diagnostics/GetDiagnosticsTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Diagnostics/GetDiagnosticsTests.cs @@ -224,6 +224,27 @@ partial class Class Assert.True(completedCompilationUnits.Contains(tree1.FilePath)); } + [Fact, WorkItem(8178, "https://github.com/dotnet/roslyn/issues/8178")] + public void TestEarlyCancellation() + { + var source = @" +namespace N1 +{ + partial class Class + { + private void NonPartialMethod1() { } + partial void PartialMethod(); + } +} +"; + var tree = CSharpSyntaxTree.ParseText(source, path: "file1"); + var eventQueue = new AsyncQueue(); + var compilation = CreateCompilationWithMscorlib45(new[] { tree }).WithEventQueue(eventQueue); + eventQueue.TryComplete(); // complete the queue before the compiler is finished with it + var model = compilation.GetSemanticModel(tree); + model.GetDiagnostics(tree.GetRoot().FullSpan); + } + private static bool DequeueCompilationEvents(AsyncQueue eventQueue, out bool compilationStartedFired, out HashSet declaredSymbolNames, out HashSet completedCompilationUnits) { compilationStartedFired = false; diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/SymbolDistinguisherTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/SymbolDistinguisherTests.cs index 63adcdc10b0b2ba47e8c04cadb9027e17ba9ab3f..dbe96d96ebd7066238c68a8ee6cea7d3fac96995 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/SymbolDistinguisherTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/SymbolDistinguisherTests.cs @@ -704,6 +704,52 @@ private static bool AreEqual(SymbolDistinguisher a, SymbolDistinguisher b) return a.First.Equals(b.First) && a.Second.Equals(b.Second); } + [WorkItem(8470, "https://github.com/dotnet/roslyn/issues/8470")] + [Fact] + public void DescriptionNoCompilation() + { + var source = +@"class A { } +class B { }"; + var compilation = CreateCompilationWithMscorlib(source); + var typeA = compilation.GetMember("A"); + var typeB = compilation.GetMember("B"); + var distinguisher1 = new SymbolDistinguisher(compilation, typeA, typeB); + var distinguisher2 = new SymbolDistinguisher(null, typeA, typeB); + var arg1A = distinguisher1.First; + var arg2A = distinguisher2.First; + Assert.False(arg1A.Equals(arg2A)); + Assert.False(arg2A.Equals(arg1A)); + int hashCode1A = arg1A.GetHashCode(); + int hashCode2A = arg2A.GetHashCode(); + } + + [WorkItem(8470, "https://github.com/dotnet/roslyn/issues/8470")] + [Fact] + public void CompareDiagnosticsNoCompilation() + { + var source1 = +@"public class A { } +public class B where T : A { }"; + var compilation1 = CreateCompilationWithMscorlib(source1); + compilation1.VerifyDiagnostics(); + var ref1 = compilation1.EmitToImageReference(); + var source2 = +@"class C : B { }"; + var compilation2 = CreateCompilationWithMscorlib(source2, references: new[] { ref1 }); + var diagnostics = compilation2.GetDiagnostics(); + diagnostics.Verify( + // (1,7): error CS0311: The type 'object' cannot be used as type parameter 'T' in the generic type or method 'B'. There is no implicit reference conversion from 'object' to 'A'. + // class C : B { } + Diagnostic(ErrorCode.ERR_GenericConstraintNotSatisfiedRefType, "C").WithArguments("B", "A", "T", "object").WithLocation(1, 7)); + // Command-line compiler calls SymbolDistinguisher.Description.GetHashCode() + // when adding diagnostics to a set. + foreach (var diagnostic in diagnostics) + { + diagnostic.GetHashCode(); + } + } + [WorkItem(8588, "https://github.com/dotnet/roslyn/issues/8588")] [Fact] public void SameErrorTypeArgumentsDifferentSourceAssemblies() diff --git a/src/Compilers/Core/CodeAnalysisTest/AsyncQueueTests.cs b/src/Compilers/Core/CodeAnalysisTest/AsyncQueueTests.cs index 4860ece8606a63814cad6dc4f72f069e3567243f..0a3bccbb69b97c952a8e0e5f2df2a2afc5e3ddc5 100644 --- a/src/Compilers/Core/CodeAnalysisTest/AsyncQueueTests.cs +++ b/src/Compilers/Core/CodeAnalysisTest/AsyncQueueTests.cs @@ -43,6 +43,17 @@ public void TryEnqueueAfterComplete() Assert.False(queue.TryEnqueue(42)); } + [Fact] + public void TryEnqueueAfterPromisingNotTo() + { + var queue = new AsyncQueue(); + Assert.True(queue.TryEnqueue(42)); + queue.PromiseNotToEnqueue(); + Assert.Throws(typeof(InvalidOperationException), () => { + queue.TryEnqueue(42); + }); + } + [Fact] public async Task DequeueThenEnqueue() { diff --git a/src/Compilers/Core/CodeAnalysisTest/Diagnostics/AnalysisContextInfoTests.cs b/src/Compilers/Core/CodeAnalysisTest/Diagnostics/AnalysisContextInfoTests.cs index d2f2b2d761517efe4e65c6beba729957e1b92416..e0d5288ecfa33afa6fc4545d397189b3f42d8b3a 100644 --- a/src/Compilers/Core/CodeAnalysisTest/Diagnostics/AnalysisContextInfoTests.cs +++ b/src/Compilers/Core/CodeAnalysisTest/Diagnostics/AnalysisContextInfoTests.cs @@ -1,7 +1,9 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; using System.Collections.Immutable; +using System.Linq; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Semantics; diff --git a/src/Compilers/Core/Portable/CodeAnalysisResources.Designer.cs b/src/Compilers/Core/Portable/CodeAnalysisResources.Designer.cs index f9f7f82438851aaf45da9bcbf778aa7860dd197d..2b4fcc56b8d84baf02c2ac7380b3b71c08aa838f 100644 --- a/src/Compilers/Core/Portable/CodeAnalysisResources.Designer.cs +++ b/src/Compilers/Core/Portable/CodeAnalysisResources.Designer.cs @@ -1073,7 +1073,7 @@ internal class CodeAnalysisResources { } /// - /// Looks up a localized string similar to Then span does not include the end of a line.. + /// Looks up a localized string similar to The span does not include the end of a line.. /// internal static string SpanDoesNotIncludeEndOfLine { get { diff --git a/src/Compilers/Core/Portable/Compilation/SemanticModel.cs b/src/Compilers/Core/Portable/Compilation/SemanticModel.cs index 7e657913010e0598efd6db4c67dce62ae96e8ffe..7b5c0f68fdbc929390f3ba2a47e2704319779272 100644 --- a/src/Compilers/Core/Portable/Compilation/SemanticModel.cs +++ b/src/Compilers/Core/Portable/Compilation/SemanticModel.cs @@ -1,5 +1,6 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; @@ -71,7 +72,7 @@ public SyntaxTree SyntaxTree /// public IOperation GetOperation(SyntaxNode node, CancellationToken cancellationToken = default(CancellationToken)) { - return this.GetOperationCore(node, cancellationToken); + return GetOperationCore(node, cancellationToken); } protected abstract IOperation GetOperationCore(SyntaxNode node, CancellationToken cancellationToken); diff --git a/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalysisState.cs b/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalysisState.cs index a642d24adc3f7b89ee344ce2305d053f35cca5ef..62f68997a896b314d39001db922b947035724e18 100644 --- a/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalysisState.cs +++ b/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalysisState.cs @@ -196,7 +196,7 @@ private PerAnalyzerState GetAnalyzerState(DiagnosticAnalyzer analyzer) var fullSpan = tree.GetRoot(cancellationToken).FullSpan; var declarationInfos = new List(); model.ComputeDeclarationsInSpan(fullSpan, getSymbol: true, builder: declarationInfos, cancellationToken: cancellationToken); - return declarationInfos.Select(declInfo => declInfo.DeclaredSymbol).WhereNotNull(); + return declarationInfos.Select(declInfo => declInfo.DeclaredSymbol).Distinct().WhereNotNull(); } private static ImmutableArray CreateCompilationEventsForTree(IEnumerable declaredSymbols, SyntaxTree tree, Compilation compilation) diff --git a/src/Compilers/Core/Portable/DiagnosticAnalyzer/AsyncQueue.cs b/src/Compilers/Core/Portable/DiagnosticAnalyzer/AsyncQueue.cs index 85247ed7db419bfa8e5fb5958d581ca00b0a7646..54274a1a325501363f38acf6e4e08ba8f9d1a35d 100644 --- a/src/Compilers/Core/Portable/DiagnosticAnalyzer/AsyncQueue.cs +++ b/src/Compilers/Core/Portable/DiagnosticAnalyzer/AsyncQueue.cs @@ -24,6 +24,7 @@ internal sealed class AsyncQueue private readonly Queue _data = new Queue(); private Queue> _waiters; private bool _completed; + private bool _disallowEnqueue; private object SyncObject { @@ -70,6 +71,11 @@ public bool TryEnqueue(TElement value) private bool EnqueueCore(TElement value) { + if (_disallowEnqueue) + { + throw new InvalidOperationException($"Cannot enqueue data after PromiseNotToEnqueue."); + } + TaskCompletionSource waiter; lock (SyncObject) { @@ -140,6 +146,11 @@ public void Complete() } } + public void PromiseNotToEnqueue() + { + _disallowEnqueue = true; + } + /// /// Same operation as except it will not /// throw if the queue is already completed. diff --git a/src/Compilers/Core/Portable/DiagnosticAnalyzer/CompilationWithAnalyzers.cs b/src/Compilers/Core/Portable/DiagnosticAnalyzer/CompilationWithAnalyzers.cs index 1d421d4bc84067e486f7a74c84948a003b19823d..16a20a64cfe18ecb32c361777c421e0b215a0273 100644 --- a/src/Compilers/Core/Portable/DiagnosticAnalyzer/CompilationWithAnalyzers.cs +++ b/src/Compilers/Core/Portable/DiagnosticAnalyzer/CompilationWithAnalyzers.cs @@ -904,7 +904,7 @@ private AsyncQueue GetPendingEvents(ImmutableArray GetPendingEvents(ImmutableArray - ''' Analyses method body for error conditions such as definite assignments, unreachable code etc... + ''' Analyzes method body for error conditions such as definite assignments, unreachable code etc... ''' ''' This analysis is done when doing the full compile or when responding to GetCompileDiagnostics. ''' This method assume that the trees are already bound and will not do any rewriting/lowering diff --git a/src/Compilers/VisualBasic/Portable/CodeGen/Optimizer/StackScheduler.Analyzer.vb b/src/Compilers/VisualBasic/Portable/CodeGen/Optimizer/StackScheduler.Analyzer.vb index e4b835f3e46ff907f5e09ad40404ba4112dcf2a7..fa8e0c951f1fd3b770539612d68c844fafb7cd34 100644 --- a/src/Compilers/VisualBasic/Portable/CodeGen/Optimizer/StackScheduler.Analyzer.vb +++ b/src/Compilers/VisualBasic/Portable/CodeGen/Optimizer/StackScheduler.Analyzer.vb @@ -27,7 +27,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeGen End Enum ''' - ''' Analyses the tree trying to figure which locals may live on stack. It is + ''' Analyzes the tree trying to figure which locals may live on stack. It is ''' a fairly delicate process and must be very familiar with how CodeGen works. ''' It is essentially a part of CodeGen. ''' diff --git a/src/Compilers/VisualBasic/Portable/Compilation/MethodCompiler.vb b/src/Compilers/VisualBasic/Portable/Compilation/MethodCompiler.vb index 7edf987a6ce2d09830910e99e17aea8f85b25e4d..a9576825a2ffe7e1c1555b082e041fe878b56f3b 100644 --- a/src/Compilers/VisualBasic/Portable/Compilation/MethodCompiler.vb +++ b/src/Compilers/VisualBasic/Portable/Compilation/MethodCompiler.vb @@ -1196,7 +1196,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic Return semanticModel End Function) Dim symbolToProduce = If(method.PartialDefinitionPart, method) - compilation.EventQueue.Enqueue(New SymbolDeclaredCompilationEvent(compilation, symbolToProduce, lazySemanticModel)) + compilation.EventQueue.TryEnqueue(New SymbolDeclaredCompilationEvent(compilation, symbolToProduce, lazySemanticModel)) End If End If End If diff --git a/src/Compilers/VisualBasic/Portable/Compilation/VisualBasicCompilation.vb b/src/Compilers/VisualBasic/Portable/Compilation/VisualBasicCompilation.vb index 20483c20b0f88e80a4122b2403f12e5b7fd9674b..adfd70b3edb9ad910a5500958ceb9d7637fa2ec3 100644 --- a/src/Compilers/VisualBasic/Portable/Compilation/VisualBasicCompilation.vb +++ b/src/Compilers/VisualBasic/Portable/Compilation/VisualBasicCompilation.vb @@ -448,7 +448,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic Debug.Assert(_lazyAssemblySymbol Is Nothing) If Me.EventQueue IsNot Nothing Then - Me.EventQueue.Enqueue(New CompilationStartedEvent(Me)) + Me.EventQueue.TryEnqueue(New CompilationStartedEvent(Me)) End If End Sub @@ -1649,12 +1649,13 @@ Namespace Microsoft.CodeAnalysis.VisualBasic End SyncLock If completedCompilationUnit Then - EventQueue.Enqueue(New CompilationUnitCompletedEvent(Me, tree)) + EventQueue.TryEnqueue(New CompilationUnitCompletedEvent(Me, tree)) End If If completedCompilation Then - EventQueue.Enqueue(New CompilationCompletedEvent(Me)) - EventQueue.Complete() ' signal the End Of compilation events + EventQueue.TryEnqueue(New CompilationCompletedEvent(Me)) + EventQueue.PromiseNotToEnqueue() + EventQueue.TryComplete() ' signal the End Of compilation events End If End Sub @@ -1675,7 +1676,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic Friend Sub SymbolDeclaredEvent(symbol As Symbol) If ShouldAddEvent(symbol) Then - EventQueue.Enqueue(New SymbolDeclaredCompilationEvent(Me, symbol)) + EventQueue.TryEnqueue(New SymbolDeclaredCompilationEvent(Me, symbol)) End If End Sub diff --git a/src/Compilers/VisualBasic/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.vb b/src/Compilers/VisualBasic/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.vb index a14275457ab79f3663cf4b3aa33c34b9d0df16a6..e007bec19c935eb16d51aed01add67ccc49e629d 100644 --- a/src/Compilers/VisualBasic/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.vb +++ b/src/Compilers/VisualBasic/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.vb @@ -130,7 +130,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic End Sub ''' - ''' Analyses method body that belongs to the given method symbol. + ''' Analyzes method body that belongs to the given method symbol. ''' Public Shared Function AnalyzeMethodBody(node As BoundBlock, method As MethodSymbol, symbolsCapturedWithoutCtor As ISet(Of Symbol), diagnostics As DiagnosticBag) As Analysis Debug.Assert(Not node.HasErrors) diff --git a/src/Compilers/VisualBasic/Portable/Parser/ParserFeature.vb b/src/Compilers/VisualBasic/Portable/Parser/ParserFeature.vb index 3e513754a43dda0ad75f13f81a9defd2297011d7..e0602fb01d704f20c0b4ca17d040501f53079b91 100644 --- a/src/Compilers/VisualBasic/Portable/Parser/ParserFeature.vb +++ b/src/Compilers/VisualBasic/Portable/Parser/ParserFeature.vb @@ -40,7 +40,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Syntax.InternalSyntax Case Feature.DigitSeparators Return "digitSeparators" - Case Feature.BinaryLiterals + Case feature.BinaryLiterals Return "binaryLiterals" Case Else diff --git a/src/Compilers/VisualBasic/Test/Emit/CodeGen/CodeGenClosureLambdaTests.vb b/src/Compilers/VisualBasic/Test/Emit/CodeGen/CodeGenClosureLambdaTests.vb index 30d57027e002390907c5792407cb4b4661fdab3b..65ada1bba31d88e75eb94cd0a9b80e5ccb40354e 100644 --- a/src/Compilers/VisualBasic/Test/Emit/CodeGen/CodeGenClosureLambdaTests.vb +++ b/src/Compilers/VisualBasic/Test/Emit/CodeGen/CodeGenClosureLambdaTests.vb @@ -3887,6 +3887,73 @@ End Class CompileAndVerify(source) End Sub + + + Public Sub ClosureInSwitchStatementWithNullableExpression() + Dim verifier = CompileAndVerify( + + +Imports System +Class C + Shared Sub Main() + Dim i As Integer? = Nothing + Select Case i + Case 0 + Case Else + Dim o As Object = Nothing + Dim f = Function() o + Console.Write("{0}", f() Is Nothing) + End Select + End Sub +End Class + +, expectedOutput:="True") + verifier.VerifyIL("C.Main", + ) + End Sub End Class End Namespace diff --git a/src/EditorFeatures/Core/Implementation/Diagnostics/AbstractDiagnosticsTaggerProvider.TaggerProvider.cs b/src/EditorFeatures/Core/Implementation/Diagnostics/AbstractDiagnosticsTaggerProvider.TaggerProvider.cs index 6171cce5f4d7c04fc58d13995ac9969682d11152..c3cda6f9447358c0156cf287194ae0585575e0ce 100644 --- a/src/EditorFeatures/Core/Implementation/Diagnostics/AbstractDiagnosticsTaggerProvider.TaggerProvider.cs +++ b/src/EditorFeatures/Core/Implementation/Diagnostics/AbstractDiagnosticsTaggerProvider.TaggerProvider.cs @@ -124,8 +124,7 @@ private void ProduceTags(TaggerContext context, DocumentSnapshotSpan spanT { if (_owner.IncludeDiagnostic(diagnosticData)) { - var actualSpan = diagnosticData - .GetExistingOrCalculatedTextSpan(sourceText) + var actualSpan = AdjustSpan(diagnosticData.GetExistingOrCalculatedTextSpan(sourceText), sourceText) .ToSnapshotSpan(editorSnapshot) .TranslateTo(requestedSnapshot, SpanTrackingMode.EdgeExclusive); @@ -142,6 +141,21 @@ private void ProduceTags(TaggerContext context, DocumentSnapshotSpan spanT } } + private TextSpan AdjustSpan(TextSpan span, SourceText text) + { + var start = Math.Max(0, Math.Min(span.Start, text.Length)); + var end = Math.Max(0, Math.Min(span.End, text.Length)); + + if (start > end) + { + var temp = end; + end = start; + start = temp; + } + + return TextSpan.FromBounds(start, end); + } + private bool IsSuppressed(NormalizedSnapshotSpanCollection suppressedSpans, SnapshotSpan span) { return suppressedSpans != null && suppressedSpans.IntersectsWith(span); diff --git a/src/EditorFeatures/Core/Implementation/Formatting/FormatCommandHandler.cs b/src/EditorFeatures/Core/Implementation/Formatting/FormatCommandHandler.cs index d3344743321ce5768a1b0ec1eb469fd38cc9fde9..b5aa922b7f93c3677175d8720eed9e42f4b0aa6d 100644 --- a/src/EditorFeatures/Core/Implementation/Formatting/FormatCommandHandler.cs +++ b/src/EditorFeatures/Core/Implementation/Formatting/FormatCommandHandler.cs @@ -49,6 +49,7 @@ private void Format(ITextView textView, Document document, TextSpan? selectionOp { var formattingService = document.GetLanguageService(); + using (Logger.LogBlock(FunctionId.CommandHandler_FormatCommand, cancellationToken)) using (var transaction = new CaretPreservingEditTransaction(EditorFeaturesResources.Formatting, textView, _undoHistoryRegistry, _editorOperationsFactoryService)) { var changes = formattingService.GetFormattingChangesAsync(document, selectionOpt, cancellationToken).WaitAndGetResult(cancellationToken); diff --git a/src/EditorFeatures/Core/Implementation/InlineRename/InlineRenameSession.OpenTextBufferManager.cs b/src/EditorFeatures/Core/Implementation/InlineRename/InlineRenameSession.OpenTextBufferManager.cs index 3a19737204ab6819099c3873e0ec1ce34b97576d..10040026ce111b8c62933bf6c0f68a59673d3eb3 100644 --- a/src/EditorFeatures/Core/Implementation/InlineRename/InlineRenameSession.OpenTextBufferManager.cs +++ b/src/EditorFeatures/Core/Implementation/InlineRename/InlineRenameSession.OpenTextBufferManager.cs @@ -198,7 +198,11 @@ internal void SetReferenceSpans(IEnumerable spans) _activeSpan = _activeSpan.HasValue && spans.Contains(_activeSpan.Value) ? _activeSpan - : spans.FirstOrNullable(); + : spans.Where(s => + // in tests `ActiveTextview` can be null so don't depend on it + ActiveTextview == null || + ActiveTextview.GetSpanInView(_subjectBuffer.CurrentSnapshot.GetSpan(s.ToSpan())).Count != 0) // spans were successfully projected + .FirstOrNullable(); // filter to spans that have a projection UpdateReadOnlyRegions(); this.ApplyReplacementText(updateSelection: false); @@ -245,10 +249,31 @@ private void OnTextBufferChanged(object sender, TextContentChangedEventArgs args } } + /// + /// This is a work around for a bug in Razor where the projection spans can get out-of-sync with the + /// identifiers. When that bug is fixed this helper can be deleted. + /// + private bool AreAllReferenceSpansMappable() + { + // in tests `ActiveTextview` could be null so don't depend on it + return ActiveTextview == null || + _referenceSpanToLinkedRenameSpanMap.Keys + .Select(s => s.ToSpan()) + .All(s => + s.End <= _subjectBuffer.CurrentSnapshot.Length && // span is valid for the snapshot + ActiveTextview.GetSpanInView(_subjectBuffer.CurrentSnapshot.GetSpan(s)).Count != 0); // spans were successfully projected + } + internal void ApplyReplacementText(bool updateSelection = true) { AssertIsForeground(); + if (!AreAllReferenceSpansMappable()) + { + // don't dynamically update the reference spans for documents with unmappable projections + return; + } + _session.UndoManager.ApplyCurrentState( _subjectBuffer, s_propagateSpansEditTag, @@ -287,6 +312,12 @@ internal void ApplyConflictResolutionEdits(IInlineRenameReplacementInfo conflict { AssertIsForeground(); + if (!AreAllReferenceSpansMappable()) + { + // don't dynamically update the reference spans for documents with unmappable projections + return; + } + using (new SelectionTracking(this)) { // 1. Undo any previous edits and update the buffer to resulting document after conflict resolution @@ -553,8 +584,10 @@ public SelectionTracking(OpenTextBufferManager openTextBufferManager) var containingSpans = openTextBufferManager._referenceSpanToLinkedRenameSpanMap.Select(kvp => { - var ss = textView.GetSpanInView(kvp.Value.TrackingSpan.GetSpan(snapshot)).Single(); - if (ss.IntersectsWith(selection.ActivePoint.Position) || ss.IntersectsWith(selection.AnchorPoint.Position)) + // GetSpanInView() can return an empty collection if the tracking span isn't mapped to anything + // in the current view, specifically a `@model SomeModelClass` directive in a Razor file. + var ss = textView.GetSpanInView(kvp.Value.TrackingSpan.GetSpan(snapshot)).FirstOrDefault(); + if (ss != null && (ss.IntersectsWith(selection.ActivePoint.Position) || ss.IntersectsWith(selection.AnchorPoint.Position))) { return Tuple.Create(kvp.Key, ss); } diff --git a/src/EditorFeatures/Test/Diagnostics/DiagnosticAnalyzerServiceTests.cs b/src/EditorFeatures/Test/Diagnostics/DiagnosticAnalyzerServiceTests.cs new file mode 100644 index 0000000000000000000000000000000000000000..7b2659674e7311d1b912d6e84c5b3719c2c9f935 --- /dev/null +++ b/src/EditorFeatures/Test/Diagnostics/DiagnosticAnalyzerServiceTests.cs @@ -0,0 +1,133 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Immutable; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Shared.TestHooks; +using Microsoft.CodeAnalysis.Text; +using Roslyn.Test.Utilities; +using Roslyn.Utilities; +using Xunit; + +namespace Microsoft.CodeAnalysis.Editor.UnitTests.Diagnostics +{ + public class DiagnosticAnalyzerServiceTests + { + [Fact] + public async Task TestHasSuccessfullyLoadedBeingFalse() + { + var workspace = new AdhocWorkspace(); + var document = GetDocumentFromIncompleteProject(workspace); + + // create listener/service/analyzer + var listener = new AsynchronousOperationListener(); + var service = new MyDiagnosticAnalyzerService(new Analyzer(), listener); + var analyzer = service.CreateIncrementalAnalyzer(workspace); + + // listen to events + // check empty since this could be called to clear up existing diagnostics + service.DiagnosticsUpdated += (s, a) => Assert.Empty(a.Diagnostics); + + // now call each analyze method. none of them should run. + await analyzer.AnalyzeSyntaxAsync(document, CancellationToken.None).ConfigureAwait(false); + await analyzer.AnalyzeDocumentAsync(document, bodyOpt: null, cancellationToken: CancellationToken.None).ConfigureAwait(false); + await analyzer.AnalyzeProjectAsync(document.Project, semanticsChanged: true, cancellationToken: CancellationToken.None).ConfigureAwait(false); + + // wait for all events to raised + await listener.CreateWaitTask().ConfigureAwait(false); + } + + [Fact] + public async Task TestHasSuccessfullyLoadedBeingFalseWhenFileOpened() + { + var workspace = new AdhocWorkspace(); + var document = GetDocumentFromIncompleteProject(workspace); + + // open document + workspace.OpenDocument(document.Id); + + // create listener/service/analyzer + var listener = new AsynchronousOperationListener(); + var service = new MyDiagnosticAnalyzerService(new Analyzer(), listener); + var analyzer = service.CreateIncrementalAnalyzer(workspace); + + bool syntax = false; + bool semantic = false; + + // listen to events + service.DiagnosticsUpdated += (s, a) => + { + switch (a.Diagnostics.Length) + { + case 0: + return; + case 1: + syntax |= a.Diagnostics[0].Id == Analyzer.s_syntaxRule.Id; + semantic |= a.Diagnostics[0].Id == Analyzer.s_semanticRule.Id; + return; + default: + AssertEx.Fail("shouldn't reach here"); + return; + } + }; + + // now call each analyze method. none of them should run. + await analyzer.AnalyzeSyntaxAsync(document, CancellationToken.None).ConfigureAwait(false); + await analyzer.AnalyzeDocumentAsync(document, bodyOpt: null, cancellationToken: CancellationToken.None).ConfigureAwait(false); + await analyzer.AnalyzeProjectAsync(document.Project, semanticsChanged: true, cancellationToken: CancellationToken.None).ConfigureAwait(false); + + // wait for all events to raised + await listener.CreateWaitTask().ConfigureAwait(false); + + // two should have been called. + Assert.True(syntax); + Assert.True(semantic); + } + + private static Document GetDocumentFromIncompleteProject(AdhocWorkspace workspace) + { + var project = workspace.AddProject( + ProjectInfo.Create( + ProjectId.CreateNewId(), + VersionStamp.Create(), + "CSharpProject", + "CSharpProject", + LanguageNames.CSharp).WithHasAllInformation(hasAllInformation: false)); + + return workspace.AddDocument(project.Id, "Empty.cs", SourceText.From("")); + } + + private class MyDiagnosticAnalyzerService : DiagnosticAnalyzerService + { + internal MyDiagnosticAnalyzerService(DiagnosticAnalyzer analyzer, IAsynchronousOperationListener listener) + : base(new HostAnalyzerManager( + ImmutableArray.Create( + new TestAnalyzerReferenceByLanguage( + ImmutableDictionary>.Empty.Add(LanguageNames.CSharp, ImmutableArray.Create(analyzer)))), + hostDiagnosticUpdateSource: null), + hostDiagnosticUpdateSource: null, + registrationService: new MockDiagnosticUpdateSourceRegistrationService(), + listener: listener) + { + } + } + + private class Analyzer : DiagnosticAnalyzer + { + internal static readonly DiagnosticDescriptor s_syntaxRule = new DiagnosticDescriptor("syntax", "test", "test", "test", DiagnosticSeverity.Error, isEnabledByDefault: true); + internal static readonly DiagnosticDescriptor s_semanticRule = new DiagnosticDescriptor("semantic", "test", "test", "test", DiagnosticSeverity.Error, isEnabledByDefault: true); + internal static readonly DiagnosticDescriptor s_compilationRule = new DiagnosticDescriptor("compilation", "test", "test", "test", DiagnosticSeverity.Error, isEnabledByDefault: true); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(s_syntaxRule, s_semanticRule, s_compilationRule); + + public override void Initialize(AnalysisContext context) + { + context.RegisterSyntaxTreeAction(c => c.ReportDiagnostic(Diagnostic.Create(s_syntaxRule, c.Tree.GetRoot().GetLocation()))); + context.RegisterSemanticModelAction(c => c.ReportDiagnostic(Diagnostic.Create(s_semanticRule, c.SemanticModel.SyntaxTree.GetRoot().GetLocation()))); + context.RegisterCompilationAction(c => c.ReportDiagnostic(Diagnostic.Create(s_compilationRule, c.Compilation.SyntaxTrees.First().GetRoot().GetLocation()))); + } + } + } +} diff --git a/src/EditorFeatures/Test/Diagnostics/DiagnosticsSquiggleTaggerProviderTests.cs b/src/EditorFeatures/Test/Diagnostics/DiagnosticsSquiggleTaggerProviderTests.cs index d725afb57024b4c835dfffed4a25e35a298347a5..938a528c0574b5ebeebe6751624b0c4413fd96e5 100644 --- a/src/EditorFeatures/Test/Diagnostics/DiagnosticsSquiggleTaggerProviderTests.cs +++ b/src/EditorFeatures/Test/Diagnostics/DiagnosticsSquiggleTaggerProviderTests.cs @@ -75,7 +75,7 @@ internal class DiagnosticTaggerWrapper : IDisposable if (analyzerMap != null || updateSource == null) { - AnalyzerService = CreateDiagnosticAnalyzerService(analyzerMap, new AggregateAsynchronousOperationListener(_listeners, FeatureAttribute.DiagnosticService)); + AnalyzerService = CreateDiagnosticAnalyzerService(analyzerMap, _asyncListener); } if (updateSource == null) diff --git a/src/EditorFeatures/Test/EditorServicesTest.csproj b/src/EditorFeatures/Test/EditorServicesTest.csproj index a4ab743043fb3620f92b13d2e9ab3b72ed5bb197..76b2c4e835592ee51de4857142de14bfe99d6368 100644 --- a/src/EditorFeatures/Test/EditorServicesTest.csproj +++ b/src/EditorFeatures/Test/EditorServicesTest.csproj @@ -229,6 +229,7 @@ + diff --git a/src/EditorFeatures/Test/Workspaces/TestWorkspace_XmlConsumption.cs b/src/EditorFeatures/Test/Workspaces/TestWorkspace_XmlConsumption.cs index b19e552eeaa964bddf251987562d5d7e8762e99e..261d7cf42fdb54f6bb6ca661c228f5fdda4df52d 100644 --- a/src/EditorFeatures/Test/Workspaces/TestWorkspace_XmlConsumption.cs +++ b/src/EditorFeatures/Test/Workspaces/TestWorkspace_XmlConsumption.cs @@ -382,7 +382,11 @@ private static ParseOptions GetParseOptionsWithFeatures(ParseOptions parseOption var features = entries.Select(x => { var split = x.Split('='); - return new KeyValuePair(split[0], split[1]); + + var key = split[0]; + var value = split.Length == 2 ? split[1] : "true"; + + return new KeyValuePair(key, value); }); return parseOptions.WithFeatures(features); diff --git a/src/EditorFeatures/Test2/Diagnostics/DiagnosticServiceTests.vb b/src/EditorFeatures/Test2/Diagnostics/DiagnosticServiceTests.vb index 6f995158ca22d5419d12618fb8bd847c562b20e6..1d7e6b84de81cf0f9f17ebb8fd3125ade4cc5cf1 100644 --- a/src/EditorFeatures/Test2/Diagnostics/DiagnosticServiceTests.vb +++ b/src/EditorFeatures/Test2/Diagnostics/DiagnosticServiceTests.vb @@ -809,6 +809,38 @@ class AnonymousFunctions End Using End Function + + Public Sub TestMultiplePartialDefinitionsInAFile() + Dim test = + + + partial class Foo { } + partial class Foo { } + + + + + Using workspace = TestWorkspace.CreateWorkspace(test) + Dim project = workspace.CurrentSolution.Projects.Single() + Dim analyzer = New NamedTypeAnalyzer + Dim analyzerReference = New AnalyzerImageReference(ImmutableArray.Create(Of DiagnosticAnalyzer)(analyzer)) + project = project.AddAnalyzerReference(analyzerReference) + Dim diagnosticService = New TestDiagnosticAnalyzerService() + + Dim incrementalAnalyzer = diagnosticService.CreateIncrementalAnalyzer(workspace) + Dim descriptorsMap = diagnosticService.GetDiagnosticDescriptors(project) + Assert.Equal(1, descriptorsMap.Count) + + ' Verify no duplicate analysis/diagnostics. + Dim document = project.Documents.Single() + Dim diagnostics = diagnosticService.GetDiagnosticsAsync(project.Solution, project.Id) _ + .WaitAndGetResult(CancellationToken.None) _ + .Select(Function(d) d.Id = NamedTypeAnalyzer.DiagDescriptor.Id) + + Assert.Equal(1, diagnostics.Count) + End Using + End Sub + Public Sub TestPartialTypeInGeneratedCode() Dim test = @@ -1354,6 +1386,29 @@ public class B End Sub End Class + Private Class NamedTypeAnalyzer + Inherits DiagnosticAnalyzer + + Public Shared ReadOnly DiagDescriptor As DiagnosticDescriptor = DescriptorFactory.CreateSimpleDescriptor("DummyDiagnostic") + Public Overrides ReadOnly Property SupportedDiagnostics As ImmutableArray(Of DiagnosticDescriptor) + Get + Return ImmutableArray.Create(DiagDescriptor) + End Get + End Property + + Public Overrides Sub Initialize(context As AnalysisContext) + context.RegisterCompilationStartAction(Sub(compStartContext As CompilationStartAnalysisContext) + Dim symbols = New HashSet(Of ISymbol) + compStartContext.RegisterSymbolAction(Sub(sc As SymbolAnalysisContext) + If (symbols.Contains(sc.Symbol)) Then + Throw New Exception("Duplicate symbol callback") + End If + sc.ReportDiagnostic(Diagnostic.Create(DiagDescriptor, sc.Symbol.Locations.First())) + End Sub, SymbolKind.NamedType) + End Sub) + End Sub + End Class + Private Class DummySymbolAnalyzer Inherits DiagnosticAnalyzer diff --git a/src/EditorFeatures/Test2/Rename/RenameEngineResult.vb b/src/EditorFeatures/Test2/Rename/RenameEngineResult.vb index bbea9205f2402ecedb2648ceb1f9aa306a37b6f3..d98a000871ed8c25bed3c924634e8b23b266da3c 100644 --- a/src/EditorFeatures/Test2/Rename/RenameEngineResult.vb +++ b/src/EditorFeatures/Test2/Rename/RenameEngineResult.vb @@ -111,7 +111,7 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.Rename ' Ensure we don't have a partial solution. This is to detect for possible root causes of ' https://github.com/dotnet/roslyn/issues/9298 - If currentSolution.Projects.Any(Function(p) Not p.HasCompleteReferencesAsync().Result) Then + If currentSolution.Projects.Any(Function(p) Not p.HasSuccessfullyLoadedAsync().Result) Then AssertEx.Fail("We have an incomplete project floating around which we should not.") End If End Sub diff --git a/src/EditorFeatures/Text/Extensions.SnapshotSourceText.cs b/src/EditorFeatures/Text/Extensions.SnapshotSourceText.cs index f60a70165db3126c3b2fb44c335b9f49ffbfc3fa..f8c7668459e194f5b4d1158f84e5e8b8a2dd6623 100644 --- a/src/EditorFeatures/Text/Extensions.SnapshotSourceText.cs +++ b/src/EditorFeatures/Text/Extensions.SnapshotSourceText.cs @@ -8,6 +8,7 @@ using System.Runtime.CompilerServices; using System.Text; using System.Threading; +using Microsoft.CodeAnalysis.ErrorReporting; using Microsoft.CodeAnalysis.Internal.Log; using Microsoft.CodeAnalysis.Text; using Microsoft.CodeAnalysis.Text.Shared.Extensions; @@ -88,7 +89,27 @@ private static SnapshotSourceText CreateText(ITextSnapshot editorSnapshot) var text = strongBox.Value; if (text != null && text._reiteratedVersion == editorSnapshot.Version.ReiteratedVersionNumber) { - return text; + if (text.Length == editorSnapshot.Length) + { + return text; + } + else + { + // In editors with non-compliant Undo/Redo implementations, you can end up + // with two Versions with the same ReiteratedVersionNumber but with very + // different text. We've never provably seen this problem occur in Visual + // Studio, but we have seen crashes that look like they could have been + // caused by incorrect results being returned from this cache. + try + { + throw new InvalidOperationException( + $"The matching cached SnapshotSourceText with = <{text._reiteratedVersion}, {text.Length}> " + + $"does not match the given editorSnapshot with <{editorSnapshot.Version.ReiteratedVersionNumber}, {editorSnapshot.Length}>"); + } + catch (Exception e) when (FatalError.ReportWithoutCrash(e)) + { + } + } } text = new SnapshotSourceText(editorSnapshot, editorSnapshot.TextBuffer.GetEncodingOrUTF8()); diff --git a/src/Features/Core/Portable/Diagnostics/Analyzers/IDEDiagnosticIds.cs b/src/Features/Core/Portable/Diagnostics/Analyzers/IDEDiagnosticIds.cs index 6a9a7e91af9a4dd2284d514a077a29099d032838..c6079fac33f44cbe0f594abfc3ca716441e4a4bc 100644 --- a/src/Features/Core/Portable/Diagnostics/Analyzers/IDEDiagnosticIds.cs +++ b/src/Features/Core/Portable/Diagnostics/Analyzers/IDEDiagnosticIds.cs @@ -12,6 +12,7 @@ internal static class IDEDiagnosticIds public const string AddQualificationDiagnosticId = "IDE0006"; public const string UseImplicitTypingDiagnosticId = "IDE0007"; public const string UseExplicitTypingDiagnosticId = "IDE0008"; + public const string IntellisenseBuildFailedDiagnosticId = "IDE0006"; // Analyzer error Ids public const string AnalyzerChangedId = "IDE1001"; diff --git a/src/Features/Core/Portable/Diagnostics/BaseDiagnosticIncrementalAnalyzer.cs b/src/Features/Core/Portable/Diagnostics/BaseDiagnosticIncrementalAnalyzer.cs index b5092da4192c3433d6833f73ae3b925c9d204164..c757cada1569f6195604eddd91531035f30400b1 100644 --- a/src/Features/Core/Portable/Diagnostics/BaseDiagnosticIncrementalAnalyzer.cs +++ b/src/Features/Core/Portable/Diagnostics/BaseDiagnosticIncrementalAnalyzer.cs @@ -9,6 +9,7 @@ using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.SolutionCrawler; using Microsoft.CodeAnalysis.Text; +using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.Diagnostics { @@ -16,6 +17,8 @@ internal abstract class BaseDiagnosticIncrementalAnalyzer : IIncrementalAnalyzer { protected BaseDiagnosticIncrementalAnalyzer(DiagnosticAnalyzerService owner, Workspace workspace, HostAnalyzerManager hostAnalyzerManager, AbstractHostDiagnosticUpdateSource hostDiagnosticUpdateSource) { + Contract.ThrowIfNull(owner); + this.Owner = owner; this.Workspace = workspace; this.HostAnalyzerManager = hostAnalyzerManager; @@ -27,7 +30,7 @@ protected BaseDiagnosticIncrementalAnalyzer(DiagnosticAnalyzerService owner, Wor /// /// Analyze a single document such that local diagnostics for that document become available, /// prioritizing analyzing this document over analyzing the rest of the project. - /// Calls for each + /// Calls for each /// unique group of diagnostics, where a group is identified by analysis classification (syntax/semantics), document, and analyzer. /// /// The document to analyze. @@ -37,7 +40,7 @@ protected BaseDiagnosticIncrementalAnalyzer(DiagnosticAnalyzerService owner, Wor public abstract Task AnalyzeDocumentAsync(Document document, SyntaxNode bodyOpt, CancellationToken cancellationToken); /// /// Analyze a single project such that diagnostics for the entire project become available. - /// Calls for each + /// Calls for each /// unique group of diagnostics, where a group is identified by analysis classification (project), project, and analyzer. /// /// The project to analyze. @@ -47,7 +50,7 @@ protected BaseDiagnosticIncrementalAnalyzer(DiagnosticAnalyzerService owner, Wor public abstract Task AnalyzeProjectAsync(Project project, bool semanticsChanged, CancellationToken cancellationToken); /// /// Apply syntax tree actions (that have not already been applied) to a document. - /// Calls for each + /// Calls for each /// unique group of diagnostics, where a group is identified by analysis classification (syntax), document, and analyzer. /// /// The document to analyze. @@ -87,14 +90,14 @@ protected BaseDiagnosticIncrementalAnalyzer(DiagnosticAnalyzerService owner, Wor /// /// Flush diagnostics produced by a prior analysis of a document, /// and suppress future analysis of the document. - /// Calls with an empty collection. + /// Calls with an empty collection. /// /// public abstract void RemoveDocument(DocumentId documentId); /// /// Flush diagnostics produced by a prior analysis of a project, /// and suppress future analysis of the project. - /// Calls with an empty collection. + /// Calls with an empty collection. /// /// public abstract void RemoveProject(ProjectId projectId); @@ -106,7 +109,7 @@ protected BaseDiagnosticIncrementalAnalyzer(DiagnosticAnalyzerService owner, Wor /// analysis classification (syntax/semantics/project), document/project, and analyzer. /// /// The solution. - /// Matched against values supplied in a to . + /// Matched against values supplied in a to . /// Flag indicating whether diagnostics with source suppressions (pragma/SuppressMessageAttribute) should be included. /// /// @@ -126,7 +129,7 @@ protected BaseDiagnosticIncrementalAnalyzer(DiagnosticAnalyzerService owner, Wor /// analysis classification (syntax/semantics/project), document/project, and analyzer. /// /// The solution. - /// Matched against values supplied in a to . + /// Matched against values supplied in a to . /// Flag indicating whether diagnostics with source suppressions (pragma/SuppressMessageAttribute) should be included. /// /// @@ -237,7 +240,7 @@ public virtual void LogAnalyzerCountSummary() // internal for testing purposes. internal Action GetOnAnalyzerException(ProjectId projectId) { - return Owner?.GetOnAnalyzerException(projectId, DiagnosticLogAggregator); + return Owner.GetOnAnalyzerException(projectId, DiagnosticLogAggregator); } } } diff --git a/src/Features/Core/Portable/Diagnostics/DiagnosticAnalyzerService_UpdateSource.cs b/src/Features/Core/Portable/Diagnostics/DiagnosticAnalyzerService_UpdateSource.cs index 2674c7c231e63a93dbbe9b82bed6c364046ef285..fefc437a36b95aeb643aa8adbcb816f8873985b6 100644 --- a/src/Features/Core/Portable/Diagnostics/DiagnosticAnalyzerService_UpdateSource.cs +++ b/src/Features/Core/Portable/Diagnostics/DiagnosticAnalyzerService_UpdateSource.cs @@ -42,17 +42,30 @@ private DiagnosticAnalyzerService(IDiagnosticUpdateSourceRegistrationService reg } } - internal void RaiseDiagnosticsUpdated(object sender, DiagnosticsUpdatedArgs args) + internal void RaiseDiagnosticsUpdated(DiagnosticsUpdatedArgs args) { - // raise serialized events + // all diagnostics events are serialized. var ev = _eventMap.GetEventHandlers>(DiagnosticsUpdatedEventName); if (ev.HasHandlers) { var asyncToken = Listener.BeginAsyncOperation(nameof(RaiseDiagnosticsUpdated)); - _eventQueue.ScheduleTask(() => - { - ev.RaiseEvent(handler => handler(sender, args)); - }).CompletesAsyncOperation(asyncToken); + _eventQueue.ScheduleTask(() => ev.RaiseEvent(handler => handler(this, args))).CompletesAsyncOperation(asyncToken); + } + } + + internal void RaiseBulkDiagnosticsUpdated(Action> eventAction) + { + // all diagnostics events are serialized. + var ev = _eventMap.GetEventHandlers>(DiagnosticsUpdatedEventName); + if (ev.HasHandlers) + { + // we do this bulk update to reduce number of tasks (with captured data) enqueued. + // we saw some "out of memory" due to us having long list of pending tasks in memory. + // this is to reduce for such case to happen. + Action raiseEvents = args => ev.RaiseEvent(handler => handler(this, args)); + + var asyncToken = Listener.BeginAsyncOperation(nameof(RaiseDiagnosticsUpdated)); + _eventQueue.ScheduleTask(() => eventAction(raiseEvents)).CompletesAsyncOperation(asyncToken); } } diff --git a/src/Features/Core/Portable/Diagnostics/DiagnosticService.cs b/src/Features/Core/Portable/Diagnostics/DiagnosticService.cs index 06adcb2e1a593a09b19bb419148473251d8e6a4a..fed74bd749ff428c393d83101dbfc021183bf694 100644 --- a/src/Features/Core/Portable/Diagnostics/DiagnosticService.cs +++ b/src/Features/Core/Portable/Diagnostics/DiagnosticService.cs @@ -57,49 +57,76 @@ public DiagnosticService([ImportMany] IEnumerable>(DiagnosticsUpdatedEventName); - if (ev.HasHandlers) + if (!RequireRunningEventTasks(source, ev)) { - var eventToken = _listener.BeginAsyncOperation(DiagnosticsUpdatedEventName); - _eventQueue.ScheduleTask(() => - { - UpdateDataMap(sender, args); - ev.RaiseEvent(handler => handler(sender, args)); - }).CompletesAsyncOperation(eventToken); + return; } + + var eventToken = _listener.BeginAsyncOperation(DiagnosticsUpdatedEventName); + _eventQueue.ScheduleTask(() => + { + if (!UpdateDataMap(source, args)) + { + // there is no change, nothing to raise events for. + return; + } + + ev.RaiseEvent(handler => handler(sender, args)); + }).CompletesAsyncOperation(eventToken); } - private void UpdateDataMap(object sender, DiagnosticsUpdatedArgs args) + private bool RequireRunningEventTasks( + IDiagnosticUpdateSource source, EventMap.EventHandlerSet> ev) { - var updateSource = sender as IDiagnosticUpdateSource; - if (updateSource == null) + // basically there are 2 cases when there is no event handler registered. + // first case is when diagnostic update source itself provide GetDiagnostics functionality. + // in that case, DiagnosticService doesn't need to track diagnostics reported. so, it bail out right away. + // second case is when diagnostic source doesn't provide GetDiagnostics functionality. + // in that case, DiagnosticService needs to track diagnostics reported. so it need to enqueue background + // work to process given data regardless whether there is event handler registered or not. + // this could be separated in 2 tasks, but we already saw cases where there are too many tasks enqueued, + // so I merged it to one. + + // if it doesn't SupportGetDiagnostics, we need to process reported data, so enqueue task. + if (!source.SupportGetDiagnostics) { - return; + return true; } - Contract.Requires(_updateSources.Contains(updateSource)); + return ev.HasHandlers; + } - // we expect someone who uses this ability to small. + private bool UpdateDataMap(IDiagnosticUpdateSource source, DiagnosticsUpdatedArgs args) + { + // we expect source who uses this ability to have small number of diagnostics. lock (_gate) { + Contract.Requires(_updateSources.Contains(source)); + // check cheap early bail out - if (args.Diagnostics.Length == 0 && !_map.ContainsKey(updateSource)) + if (args.Diagnostics.Length == 0 && !_map.ContainsKey(source)) { // no new diagnostic, and we don't have update source for it. - return; + return false; } - var list = _map.GetOrAdd(updateSource, _ => new Dictionary()); - var data = updateSource.SupportGetDiagnostics ? new Data(args) : new Data(args, args.Diagnostics); + var diagnosticDataMap = _map.GetOrAdd(source, _ => new Dictionary()); - list.Remove(data.Id); - if (list.Count == 0 && args.Diagnostics.Length == 0) + diagnosticDataMap.Remove(args.Id); + if (diagnosticDataMap.Count == 0 && args.Diagnostics.Length == 0) { - _map.Remove(updateSource); - return; + _map.Remove(source); + return true; } - list.Add(args.Id, data); + var data = source.SupportGetDiagnostics ? new Data(args) : new Data(args, args.Diagnostics); + diagnosticDataMap.Add(args.Id, data); + + return true; } } diff --git a/src/Features/Core/Portable/Diagnostics/EngineV1/DiagnosticIncrementalAnalyzer.StateSet.cs b/src/Features/Core/Portable/Diagnostics/EngineV1/DiagnosticIncrementalAnalyzer.StateSet.cs index 4cd775af2d960fd70bbe90c725ddaaeaa02a312b..24220e79a7b202651d70e3f979610a50494163ae 100644 --- a/src/Features/Core/Portable/Diagnostics/EngineV1/DiagnosticIncrementalAnalyzer.StateSet.cs +++ b/src/Features/Core/Portable/Diagnostics/EngineV1/DiagnosticIncrementalAnalyzer.StateSet.cs @@ -39,10 +39,18 @@ public DiagnosticState GetState(StateType stateType) return _state[(int)stateType]; } - public void Remove(object documentOrProjectId) + public void Remove(object documentOrProjectId, bool onlyDocumentStates = false) { + // this is to clear up states. some caller such as re-analyzing a file wants to + // reset only document related states, some like removing a file wants to clear up + // states all together. for (var stateType = 0; stateType < s_stateTypeCount; stateType++) { + if (onlyDocumentStates && stateType == (int)StateType.Project) + { + continue; + } + _state[stateType].Remove(documentOrProjectId); } } diff --git a/src/Features/Core/Portable/Diagnostics/EngineV1/DiagnosticIncrementalAnalyzer.cs b/src/Features/Core/Portable/Diagnostics/EngineV1/DiagnosticIncrementalAnalyzer.cs index b4076b9bfeb38352ce9c0377cfc07a286bc76767..f7833a13bbe75a548555ea4d11a00825c1cc39a4 100644 --- a/src/Features/Core/Portable/Diagnostics/EngineV1/DiagnosticIncrementalAnalyzer.cs +++ b/src/Features/Core/Portable/Diagnostics/EngineV1/DiagnosticIncrementalAnalyzer.cs @@ -60,14 +60,37 @@ private void OnProjectAnalyzerReferenceChanged(object sender, ProjectAnalyzerRef } // events will be automatically serialized. - ClearProjectStatesAsync(e.Project, e.Removed, CancellationToken.None); + var project = e.Project; + var stateSets = e.Removed; + + // first remove all states + foreach (var stateSet in stateSets) + { + foreach (var document in project.Documents) + { + stateSet.Remove(document.Id); + } + + stateSet.Remove(project.Id); + } + + // now raise events + Owner.RaiseBulkDiagnosticsUpdated(raiseEvents => + { + foreach (var document in project.Documents) + { + RaiseDocumentDiagnosticsRemoved(document, stateSets, includeProjectState: true, raiseEvents: raiseEvents); + } + + RaiseProjectDiagnosticsRemoved(project, stateSets, raiseEvents); + }); } public override Task DocumentOpenAsync(Document document, CancellationToken cancellationToken) { using (Logger.LogBlock(FunctionId.Diagnostics_DocumentOpen, GetOpenLogMessage, document, cancellationToken)) { - return ClearOnlyDocumentStates(document, raiseEvent: true, cancellationToken: cancellationToken); + return ClearOnlyDocumentStates(document); } } @@ -78,7 +101,7 @@ public override Task DocumentCloseAsync(Document document, CancellationToken can // we don't need the info for closed file _memberRangeMap.Remove(document.Id); - return ClearOnlyDocumentStates(document, raiseEvent: true, cancellationToken: cancellationToken); + return ClearOnlyDocumentStates(document); } } @@ -88,23 +111,38 @@ public override Task DocumentResetAsync(Document document, CancellationToken can { // clear states for re-analysis and raise events about it. otherwise, some states might not updated on re-analysis // due to our build-live de-duplication logic where we put all state in Documents state. - return ClearOnlyDocumentStates(document, raiseEvent: true, cancellationToken: cancellationToken); + return ClearOnlyDocumentStates(document); } } - private Task ClearOnlyDocumentStates(Document document, bool raiseEvent, CancellationToken cancellationToken) + private Task ClearOnlyDocumentStates(Document document) { + // since managing states and raising events are separated, it can't be cancelled. + var stateSets = _stateManager.GetStateSets(document.Project); + // we remove whatever information we used to have on document open/close and re-calculate diagnostics // we had to do this since some diagnostic analyzer changes its behavior based on whether the document is opened or not. // so we can't use cached information. - ClearDocumentStates(document, _stateManager.GetStateSets(document.Project), raiseEvent, includeProjectState: false, cancellationToken: cancellationToken); + + // clean up states + foreach (var stateSet in stateSets) + { + stateSet.Remove(document.Id, onlyDocumentStates: true); + } + + // raise events + Owner.RaiseBulkDiagnosticsUpdated(raiseEvents => + { + RaiseDocumentDiagnosticsRemoved(document, stateSets, includeProjectState: false, raiseEvents: raiseEvents); + }); return SpecializedTasks.EmptyTask; } - private bool CheckOption(Workspace workspace, string language, bool forceAnalysis) + private bool CheckOptions(Project project, bool forceAnalysis) { - if (workspace.Options.GetOption(ServiceFeatureOnOffOptions.ClosedFileDiagnostic, language) && + var workspace = project.Solution.Workspace; + if (workspace.Options.GetOption(ServiceFeatureOnOffOptions.ClosedFileDiagnostic, project.Language) && workspace.Options.GetOption(RuntimeOptions.FullSolutionAnalysis)) { return true; @@ -157,14 +195,14 @@ internal CompilationWithAnalyzers GetCompilationWithAnalyzers(Project project, C public override async Task AnalyzeSyntaxAsync(Document document, CancellationToken cancellationToken) { - await AnalyzeSyntaxAsync(document, diagnosticIds: null, skipClosedFileChecks: false, cancellationToken: cancellationToken).ConfigureAwait(false); + await AnalyzeSyntaxAsync(document, diagnosticIds: null, cancellationToken: cancellationToken).ConfigureAwait(false); } - private async Task AnalyzeSyntaxAsync(Document document, ImmutableHashSet diagnosticIds, bool skipClosedFileChecks, CancellationToken cancellationToken) + private async Task AnalyzeSyntaxAsync(Document document, ImmutableHashSet diagnosticIds, CancellationToken cancellationToken) { try { - if (!skipClosedFileChecks && !CheckOption(document.Project.Solution.Workspace, document.Project.Language, document.IsOpen())) + if (!CheckOptions(document.Project, document.IsOpen())) { return; } @@ -181,7 +219,7 @@ private async Task AnalyzeSyntaxAsync(Document document, ImmutableHashSet diagnosticIds, bool skipClosedFileChecks, CancellationToken cancellationToken) + private async Task AnalyzeDocumentAsync(Document document, SyntaxNode bodyOpt, ImmutableHashSet diagnosticIds, CancellationToken cancellationToken) { try { - if (!skipClosedFileChecks && !CheckOption(document.Project.Solution.Workspace, document.Project.Language, document.IsOpen())) + if (!CheckOptions(document.Project, document.IsOpen())) { return; } @@ -230,7 +268,7 @@ private async Task AnalyzeDocumentAsync(Document document, SyntaxNode bodyOpt, I var versions = new VersionArgument(textVersion, dataVersion, projectVersion); if (bodyOpt == null) { - await AnalyzeDocumentAsync(document, versions, diagnosticIds, skipClosedFileChecks, cancellationToken).ConfigureAwait(false); + await AnalyzeDocumentAsync(document, versions, diagnosticIds, cancellationToken).ConfigureAwait(false); } else { @@ -294,7 +332,7 @@ private async Task AnalyzeBodyDocumentAsync(Document document, SyntaxNode member } } - private async Task AnalyzeDocumentAsync(Document document, VersionArgument versions, ImmutableHashSet diagnosticIds, bool skipClosedFileChecks, CancellationToken cancellationToken) + private async Task AnalyzeDocumentAsync(Document document, VersionArgument versions, ImmutableHashSet diagnosticIds, CancellationToken cancellationToken) { try { @@ -306,7 +344,7 @@ private async Task AnalyzeDocumentAsync(Document document, VersionArgument versi foreach (var stateSet in _stateManager.GetOrUpdateStateSets(document.Project)) { - if (SkipRunningAnalyzer(document.Project.CompilationOptions, userDiagnosticDriver, openedDocument, skipClosedFileChecks, stateSet)) + if (await SkipRunningAnalyzerAsync(document.Project, stateSet.Analyzer, openedDocument, skipClosedFileCheck: false, cancellationToken: cancellationToken).ConfigureAwait(false)) { await ClearExistingDiagnostics(document, stateSet, StateType.Document, cancellationToken).ConfigureAwait(false); continue; @@ -348,8 +386,7 @@ private async Task AnalyzeProjectAsync(Project project, CancellationToken cancel { try { - // Compilation actions can report diagnostics on open files, so "documentOpened = true" - if (!CheckOption(project.Solution.Workspace, project.Language, forceAnalysis: false)) + if (!CheckOptions(project, forceAnalysis: false)) { return; } @@ -363,7 +400,7 @@ private async Task AnalyzeProjectAsync(Project project, CancellationToken cancel foreach (var stateSet in _stateManager.GetOrUpdateStateSets(project)) { // Compilation actions can report diagnostics on open files, so we skipClosedFileChecks. - if (SkipRunningAnalyzer(project.CompilationOptions, analyzerDriver, openedDocument: true, skipClosedFileChecks: true, stateSet: stateSet)) + if (await SkipRunningAnalyzerAsync(project, stateSet.Analyzer, openedDocument: false, skipClosedFileCheck: true, cancellationToken: cancellationToken).ConfigureAwait(false)) { await ClearExistingDiagnostics(project, stateSet, cancellationToken).ConfigureAwait(false); continue; @@ -391,24 +428,20 @@ private async Task AnalyzeProjectAsync(Project project, CancellationToken cancel } } - private bool SkipRunningAnalyzer( - CompilationOptions compilationOptions, - DiagnosticAnalyzerDriver userDiagnosticDriver, - bool openedDocument, - bool skipClosedFileChecks, - StateSet stateSet) + private async Task SkipRunningAnalyzerAsync(Project project, DiagnosticAnalyzer analyzer, bool openedDocument, bool skipClosedFileCheck, CancellationToken cancellationToken) { - if (Owner.IsAnalyzerSuppressed(stateSet.Analyzer, userDiagnosticDriver.Project)) + // this project is not in a good state. only continue if it is opened document. + if (!await project.HasSuccessfullyLoadedAsync(cancellationToken).ConfigureAwait(false) && !openedDocument) { return true; } - if (skipClosedFileChecks) + if (Owner.IsAnalyzerSuppressed(analyzer, project)) { - return false; + return true; } - if (ShouldRunAnalyzerForClosedFile(compilationOptions, openedDocument, stateSet.Analyzer)) + if (skipClosedFileCheck || ShouldRunAnalyzerForClosedFile(analyzer, project.CompilationOptions, openedDocument)) { return false; } @@ -463,16 +496,24 @@ public override void RemoveDocument(DocumentId documentId) { _memberRangeMap.Remove(documentId); - foreach (var stateSet in _stateManager.GetStateSets(documentId.ProjectId)) + var stateSets = _stateManager.GetStateSets(documentId.ProjectId); + + foreach (var stateSet in stateSets) { stateSet.Remove(documentId); + } + Owner.RaiseBulkDiagnosticsUpdated(raiseEvents => + { + foreach (var stateSet in stateSets) + { var solutionArgs = new SolutionArgument(null, documentId.ProjectId, documentId); for (var stateType = 0; stateType < s_stateTypeCount; stateType++) { - RaiseDiagnosticsRemoved((StateType)stateType, documentId, stateSet, solutionArgs); + RaiseDiagnosticsRemoved((StateType)stateType, documentId, stateSet, solutionArgs, raiseEvents); } } + }); } } @@ -480,16 +521,24 @@ public override void RemoveProject(ProjectId projectId) { using (Logger.LogBlock(FunctionId.Diagnostics_RemoveProject, GetRemoveLogMessage, projectId, CancellationToken.None)) { - foreach (var stateSet in _stateManager.GetStateSets(projectId)) + var stateSets = _stateManager.GetStateSets(projectId); + + foreach (var stateSet in stateSets) { stateSet.Remove(projectId); + } + + _stateManager.RemoveStateSet(projectId); + Owner.RaiseBulkDiagnosticsUpdated(raiseEvents => + { + foreach (var stateSet in stateSets) + { var solutionArgs = new SolutionArgument(null, projectId, null); - RaiseDiagnosticsRemoved(StateType.Project, projectId, stateSet, solutionArgs); + RaiseDiagnosticsRemoved(StateType.Project, projectId, stateSet, solutionArgs, raiseEvents); } + }); } - - _stateManager.RemoveStateSet(projectId); } public override async Task TryAppendDiagnosticsForSpanAsync(Document document, TextSpan range, List diagnostics, bool includeSuppressedDiagnostics = false, CancellationToken cancellationToken = default(CancellationToken)) @@ -547,20 +596,16 @@ public override bool ContainsDiagnostics(Workspace workspace, ProjectId projectI return false; } - private bool ShouldRunAnalyzerForClosedFile(CompilationOptions options, bool openedDocument, DiagnosticAnalyzer analyzer) + private bool ShouldRunAnalyzerForClosedFile(DiagnosticAnalyzer analyzer, CompilationOptions options, bool openedDocument) { // we have opened document, doesn't matter - if (openedDocument || analyzer.IsCompilerAnalyzer()) - { - return true; - } - // PERF: Don't query descriptors for compiler analyzer, always execute it. - if (analyzer.IsCompilerAnalyzer()) + if (openedDocument || analyzer.IsCompilerAnalyzer()) { return true; } + // most of analyzers, number of descriptor is quite small, so this should be cheap. return Owner.GetDiagnosticDescriptors(analyzer).Any(d => GetEffectiveSeverity(d, options) != ReportDiagnostic.Hidden); } @@ -703,34 +748,40 @@ private void RaiseDiagnosticsCreatedFromCacheIfNeeded(StateType type, Document d return; } + Owner.RaiseBulkDiagnosticsUpdated(raiseEvents => + { var removedItems = existingItems.GroupBy(d => d.DocumentId).Select(g => g.Key).Except(newItems.GroupBy(d => d.DocumentId).Select(g => g.Key)); foreach (var documentId in removedItems) { if (documentId == null) { - RaiseDiagnosticsRemoved(StateType.Project, project.Id, stateSet, new SolutionArgument(project)); + RaiseDiagnosticsRemoved(StateType.Project, project.Id, stateSet, new SolutionArgument(project), raiseEvents); continue; } var document = project.GetDocument(documentId); - var argument = documentId == null ? new SolutionArgument(null, documentId.ProjectId, documentId) : new SolutionArgument(document); - RaiseDiagnosticsRemoved(StateType.Project, documentId, stateSet, argument); + var argument = document == null ? new SolutionArgument(null, documentId.ProjectId, documentId) : new SolutionArgument(document); + RaiseDiagnosticsRemoved(StateType.Project, documentId, stateSet, argument, raiseEvents); } + }); } private void RaiseProjectDiagnosticsUpdated(Project project, StateSet stateSet, ImmutableArray diagnostics) { + Owner.RaiseBulkDiagnosticsUpdated(raiseEvents => + { var group = diagnostics.GroupBy(d => d.DocumentId); foreach (var kv in group) { if (kv.Key == null) { - RaiseDiagnosticsCreated(StateType.Project, project.Id, stateSet, new SolutionArgument(project), kv.ToImmutableArrayOrEmpty()); + RaiseDiagnosticsCreated(StateType.Project, project.Id, stateSet, new SolutionArgument(project), kv.ToImmutableArrayOrEmpty(), raiseEvents); continue; } - RaiseDiagnosticsCreated(StateType.Project, kv.Key, stateSet, new SolutionArgument(project.GetDocument(kv.Key)), kv.ToImmutableArrayOrEmpty()); + RaiseDiagnosticsCreated(StateType.Project, kv.Key, stateSet, new SolutionArgument(project.GetDocument(kv.Key)), kv.ToImmutableArrayOrEmpty(), raiseEvents); } + }); } private static ImmutableArray GetDiagnosticData(ILookup lookup, DocumentId documentId) @@ -741,38 +792,62 @@ private static ImmutableArray GetDiagnosticData(ILookup diagnostics) { - if (Owner == null) - { - return; + RaiseDiagnosticsCreated(type, key, stateSet, solution, diagnostics, Owner.RaiseDiagnosticsUpdated); } + private void RaiseDiagnosticsCreated( + StateType type, object key, StateSet stateSet, SolutionArgument solution, ImmutableArray diagnostics, Action raiseEvents) + { // get right arg id for the given analyzer var id = CreateArgumentKey(type, key, stateSet); - - Owner.RaiseDiagnosticsUpdated(this, - DiagnosticsUpdatedArgs.DiagnosticsCreated(id, Workspace, solution.Solution, solution.ProjectId, solution.DocumentId, diagnostics)); + raiseEvents(DiagnosticsUpdatedArgs.DiagnosticsCreated(id, Workspace, solution.Solution, solution.ProjectId, solution.DocumentId, diagnostics)); } - private static ArgumentKey CreateArgumentKey(StateType type, object key, StateSet stateSet) + private void RaiseDiagnosticsRemoved( + StateType type, object key, StateSet stateSet, SolutionArgument solution) { - return stateSet.ErrorSourceName != null - ? new HostAnalyzerKey(stateSet.Analyzer, type, key, stateSet.ErrorSourceName) - : new ArgumentKey(stateSet.Analyzer, type, key); + RaiseDiagnosticsRemoved(type, key, stateSet, solution, Owner.RaiseDiagnosticsUpdated); } private void RaiseDiagnosticsRemoved( - StateType type, object key, StateSet stateSet, SolutionArgument solution) + StateType type, object key, StateSet stateSet, SolutionArgument solution, Action raiseEvents) { - if (Owner == null) + // get right arg id for the given analyzer + var id = CreateArgumentKey(type, key, stateSet); + raiseEvents(DiagnosticsUpdatedArgs.DiagnosticsRemoved(id, Workspace, solution.Solution, solution.ProjectId, solution.DocumentId)); + } + + private void RaiseDocumentDiagnosticsRemoved(Document document, IEnumerable stateSets, bool includeProjectState, Action raiseEvents) + { + foreach (var stateSet in stateSets) { - return; + for (var stateType = 0; stateType < s_stateTypeCount; stateType++) + { + if (!includeProjectState && stateType == (int)StateType.Project) + { + // don't re-set project state type + continue; + } + + // raise diagnostic updated event + RaiseDiagnosticsRemoved((StateType)stateType, document.Id, stateSet, new SolutionArgument(document), raiseEvents); + } + } } - // get right arg id for the given analyzer - var id = CreateArgumentKey(type, key, stateSet); + private void RaiseProjectDiagnosticsRemoved(Project project, IEnumerable stateSets, Action raiseEvents) + { + foreach (var stateSet in stateSets) + { + RaiseDiagnosticsRemoved(StateType.Project, project.Id, stateSet, new SolutionArgument(project), raiseEvents); + } + } - Owner.RaiseDiagnosticsUpdated(this, - DiagnosticsUpdatedArgs.DiagnosticsRemoved(id, Workspace, solution.Solution, solution.ProjectId, solution.DocumentId)); + private static ArgumentKey CreateArgumentKey(StateType type, object key, StateSet stateSet) + { + return stateSet.ErrorSourceName != null + ? new HostAnalyzerKey(stateSet.Analyzer, type, key, stateSet.ErrorSourceName) + : new ArgumentKey(stateSet.Analyzer, type, key); } private ImmutableArray UpdateDocumentDiagnostics( @@ -984,78 +1059,17 @@ private static async Task> GetProjectDiagnosticsAsyn } } - private void ClearDocumentStates( - Document document, IEnumerable states, - bool raiseEvent, bool includeProjectState, - CancellationToken cancellationToken) - { - // Compiler + User diagnostics - foreach (var state in states) - { - for (var stateType = 0; stateType < s_stateTypeCount; stateType++) - { - if (!includeProjectState && stateType == (int)StateType.Project) - { - // don't re-set project state type - continue; - } - - cancellationToken.ThrowIfCancellationRequested(); - ClearDocumentState(document, state, (StateType)stateType, raiseEvent); - } - } - } - - private void ClearDocumentState(Document document, StateSet stateSet, StateType type, bool raiseEvent) - { - var state = stateSet.GetState(type); - - // remove saved info - state.Remove(document.Id); - - if (raiseEvent) - { - // raise diagnostic updated event - var documentId = document.Id; - var solutionArgs = new SolutionArgument(document); - - RaiseDiagnosticsRemoved(type, document.Id, stateSet, solutionArgs); - } - } - - private void ClearProjectStatesAsync(Project project, IEnumerable states, CancellationToken cancellationToken) - { - foreach (var document in project.Documents) - { - ClearDocumentStates(document, states, raiseEvent: true, includeProjectState: true, cancellationToken: cancellationToken); - } - - foreach (var stateSet in states) - { - cancellationToken.ThrowIfCancellationRequested(); - ClearProjectState(project, stateSet); - } - } - - private void ClearProjectState(Project project, StateSet stateSet) - { - var state = stateSet.GetState(StateType.Project); - - // remove saved cache - state.Remove(project.Id); - - // raise diagnostic updated event - var solutionArgs = new SolutionArgument(project); - RaiseDiagnosticsRemoved(StateType.Project, project.Id, stateSet, solutionArgs); - } - private async Task ClearExistingDiagnostics(Document document, StateSet stateSet, StateType type, CancellationToken cancellationToken) { var state = stateSet.GetState(type); var existingData = await state.TryGetExistingDataAsync(document, cancellationToken).ConfigureAwait(false); if (existingData?.Items.Length > 0) { - ClearDocumentState(document, stateSet, type, raiseEvent: true); + // remove saved info + state.Remove(document.Id); + + // raise diagnostic updated event + RaiseDiagnosticsRemoved(type, document.Id, stateSet, new SolutionArgument(document)); } } @@ -1065,7 +1079,11 @@ private async Task ClearExistingDiagnostics(Project project, StateSet stateSet, var existingData = await state.TryGetExistingDataAsync(project, cancellationToken).ConfigureAwait(false); if (existingData?.Items.Length > 0) { - ClearProjectState(project, stateSet); + // remove saved cache + state.Remove(project.Id); + + // raise diagnostic updated event + RaiseDiagnosticsRemoved(StateType.Project, project.Id, stateSet, new SolutionArgument(project)); } } diff --git a/src/Features/Core/Portable/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs b/src/Features/Core/Portable/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs index d0fd340f1634e07c16787076018067d8951d6e21..e2433e5c724ec8b6a04036b55c2b8487da1ee38f 100644 --- a/src/Features/Core/Portable/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs +++ b/src/Features/Core/Portable/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs @@ -61,13 +61,13 @@ public override Task NewSolutionSnapshotAsync(Solution solution, CancellationTok public override void RemoveDocument(DocumentId documentId) { - Owner.RaiseDiagnosticsUpdated(this, DiagnosticsUpdatedArgs.DiagnosticsRemoved( + Owner.RaiseDiagnosticsUpdated(DiagnosticsUpdatedArgs.DiagnosticsRemoved( ValueTuple.Create(this, documentId), Workspace, null, null, null)); } public override void RemoveProject(ProjectId projectId) { - Owner.RaiseDiagnosticsUpdated(this, DiagnosticsUpdatedArgs.DiagnosticsRemoved( + Owner.RaiseDiagnosticsUpdated(DiagnosticsUpdatedArgs.DiagnosticsRemoved( ValueTuple.Create(this, projectId), Workspace, null, null, null)); } #endregion @@ -213,14 +213,12 @@ private void RaiseEvents(Project project, ImmutableArray diagnos { if (kv.Key == null) { - Owner.RaiseDiagnosticsUpdated( - this, DiagnosticsUpdatedArgs.DiagnosticsCreated( + Owner.RaiseDiagnosticsUpdated(DiagnosticsUpdatedArgs.DiagnosticsCreated( ValueTuple.Create(this, project.Id), workspace, solution, project.Id, null, kv.ToImmutableArrayOrEmpty())); continue; } - Owner.RaiseDiagnosticsUpdated( - this, DiagnosticsUpdatedArgs.DiagnosticsCreated( + Owner.RaiseDiagnosticsUpdated(DiagnosticsUpdatedArgs.DiagnosticsCreated( ValueTuple.Create(this, kv.Key), workspace, solution, project.Id, kv.Key, kv.ToImmutableArrayOrEmpty())); } } diff --git a/src/VisualStudio/CSharp/Impl/ProjectSystemShim/CSharpProjectShim.ICSharpProjectSite.cs b/src/VisualStudio/CSharp/Impl/ProjectSystemShim/CSharpProjectShim.ICSharpProjectSite.cs index 359312726ebad589019fea6ca780ba1e46e0ed3d..be31af1b191bda3e67d384fdd7407e51e38d3b27 100644 --- a/src/VisualStudio/CSharp/Impl/ProjectSystemShim/CSharpProjectShim.ICSharpProjectSite.cs +++ b/src/VisualStudio/CSharp/Impl/ProjectSystemShim/CSharpProjectShim.ICSharpProjectSite.cs @@ -122,11 +122,7 @@ public int OnImportAddedEx(string filename, string project, CompilerOptions opti bool embedInteropTypes = optionID == CompilerOptions.OPTID_IMPORTSUSINGNOPIA; var properties = new MetadataReferenceProperties(embedInteropTypes: embedInteropTypes); - // The file may not exist, so let's return an error code. In Dev10, we were - // never called us for these at all, and the project system would immediately - // return an S_FALSE. Sadly, returning S_FALSE will convert back to S_OK, which - // would prevent this file from ever being added again. Roslyn bug 7315 for more. - return AddMetadataReferenceAndTryConvertingToProjectReferenceIfPossible(filename, properties, hResultForMissingFile: VSConstants.E_FAIL); + return AddMetadataReferenceAndTryConvertingToProjectReferenceIfPossible(filename, properties); } public void OnImportRemoved(string filename, string project) diff --git a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/AbstractProject.cs b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/AbstractProject.cs index bb7de18681744ad7ba8d01f7d74604ba680f280e..4ebe3f7c4113bd7ef9162b5b4e5bbb7873700b63 100644 --- a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/AbstractProject.cs +++ b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/AbstractProject.cs @@ -303,7 +303,7 @@ public ProjectInfo CreateProjectInfoForCurrentState() { ValidateReferences(); - return ProjectInfo.Create( + var info = ProjectInfo.Create( this.Id, this.Version, this.DisplayName, @@ -318,6 +318,8 @@ public ProjectInfo CreateProjectInfoForCurrentState() projectReferences: _projectReferences, analyzerReferences: _analyzers.Values.Select(a => a.GetReference()), additionalDocuments: _additionalDocuments.Values.Select(d => d.GetInitialState())); + + return info.WithHasAllInformation(_intellisenseBuildSucceeded); } protected ImmutableArray GetStrongNameKeyPaths() @@ -347,7 +349,7 @@ public ImmutableArray GetCurrentProjectReferences() { return ImmutableArray.CreateRange(_projectReferences); } - + public ImmutableArray GetCurrentMetadataReferences() { return ImmutableArray.CreateRange(_metadataReferences); @@ -435,7 +437,7 @@ protected void SetOptions(CompilationOptions compilationOptions, ParseOptions pa } } - protected int AddMetadataReferenceAndTryConvertingToProjectReferenceIfPossible(string filePath, MetadataReferenceProperties properties, int hResultForMissingFile) + protected int AddMetadataReferenceAndTryConvertingToProjectReferenceIfPossible(string filePath, MetadataReferenceProperties properties) { // If this file is coming from a project, then we should convert it to a project reference instead AbstractProject project; @@ -450,13 +452,50 @@ protected int AddMetadataReferenceAndTryConvertingToProjectReferenceIfPossible(s } } - if (!File.Exists(filePath)) - { - return hResultForMissingFile; - } - + // regardless whether the file exists or not, we still record it. one of reason + // we do that is some cross language p2p references might be resolved + // after they are already reported as metadata references. since we use bin path + // as a way to discover them, if we don't previously record the reference ourselves, + // cross p2p references won't be resolved as p2p references when we finally have + // all required information. + // + // it looks like + // 1. project system sometimes won't guarantee build dependency for intellisense build + // if it is cross language dependency + // 2. output path of referenced cross language project might be changed to right one + // once it is already added as a metadata reference. + // + // but this has one consequence. even if a user adds a project in the solution as + // a metadata reference explicitly, that dll will be automatically converted back to p2p + // reference. + // + // unfortunately there is no way to prevent this using information we have since, + // at this point, we don't know whether it is a metadata reference added because + // we don't have enough information yet for p2p reference or user explicitly added it + // as a metadata reference. AddMetadataReferenceCore(this.MetadataReferenceProvider.CreateMetadataReference(this, filePath, properties)); + // here, we change behavior compared to old C# language service. regardless of file being exist or not, + // we will always return S_OK. this is to support cross language p2p reference better. + // + // this should make project system to cache all cross language p2p references regardless + // whether it actually exist in disk or not. + // (see Roslyn bug 7315 for history - http://vstfdevdiv:8080/DevDiv_Projects/Roslyn/_workitems?_a=edit&id=7315) + // + // after this point, Roslyn will take care of non-exist metadata reference. + // + // But, this doesn't sovle the issue where actual metadata reference + // (not cross language p2p reference) is missing at the time project is opened. + // + // in that case, msbuild filter those actual metadata references out, so project system doesn't know + // path to the reference. since it doesn't know where dll is, it can't (or currently doesn't) + // setup file change notification either to find out when dll becomes available. + // + // at this point, user has 2 ways to recover missing metadata reference once it becomes available. + // + // one way is explicitly clicking that missing reference from solution explorer reference node. + // the other is building the project. at that point, project system will refresh references + // which will discover new dll and connect to us. once it is connected, we will take care of it. return VSConstants.S_OK; } diff --git a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/AbstractProject_IIntellisenseBuildTarget.cs b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/AbstractProject_IIntellisenseBuildTarget.cs new file mode 100644 index 0000000000000000000000000000000000000000..7dc680e5b35a63a5a30d8a80bd7b315ba86ee58b --- /dev/null +++ b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/AbstractProject_IIntellisenseBuildTarget.cs @@ -0,0 +1,85 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.IO; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Internal.Log; +using Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem.Interop; +using Roslyn.Utilities; + +namespace Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem +{ + internal partial class AbstractProject : IIntellisenseBuildTarget + { + private static readonly object s_diagnosticKey = new object(); + + // set default to true so that we maintain old behavior when project system doesn't + // implement IIntellisenseBuildTarget + private bool _intellisenseBuildSucceeded = true; + private string _intellisenseBuildFailureReason = null; + + public void SetIntellisenseBuildResult(bool succeeded, string reason) + { + // set intellisense related info + _intellisenseBuildSucceeded = succeeded; + _intellisenseBuildFailureReason = string.IsNullOrWhiteSpace(reason) ? null : reason.Trim(); + + UpdateHostDiagnostics(succeeded, reason); + + if (_pushingChangesToWorkspaceHosts) + { + // set workspace reference info + ProjectTracker.NotifyWorkspaceHosts(host => (host as IVisualStudioWorkspaceHost2)?.OnHasAllInformation(Id, succeeded)); + } + } + + private void UpdateHostDiagnostics(bool succeeded, string reason) + { + if (!succeeded) + { + // report intellisense build failure to error list + this.HostDiagnosticUpdateSource?.UpdateDiagnosticsForProject( + _id, s_diagnosticKey, SpecializedCollections.SingletonEnumerable(CreateIntellisenseBuildFailureDiagnostic(reason))); + } + else + { + // clear intellisense build failure diagnostic from error list. + this.HostDiagnosticUpdateSource?.ClearDiagnosticsForProject(_id, s_diagnosticKey); + } + } + + private DiagnosticData CreateIntellisenseBuildFailureDiagnostic(string reason) + { + // log intellisense build failure + Logger.Log(FunctionId.IntellisenseBuild_Failed, KeyValueLogMessage.Create(m => m["Reason"] = reason ?? string.Empty)); + + return new DiagnosticData( + IDEDiagnosticIds.IntellisenseBuildFailedDiagnosticId, + FeaturesResources.ErrorCategory, + ServicesVSResources.IntellisenseBuildFailedMessage, + ServicesVSResources.ResourceManager.GetString(nameof(ServicesVSResources.IntellisenseBuildFailedMessage), CodeAnalysis.Diagnostics.Extensions.s_USCultureInfo), + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + warningLevel: 0, + workspace: Workspace, + projectId: Id, + title: ServicesVSResources.IntellisenseBuildFailedTitle, + description: GetDescription(reason), + helpLink: "http://go.microsoft.com/fwlink/p/?LinkID=734719"); + } + + private string GetDescription(string reason) + { + var logFilePath = $"{Path.GetTempPath()}\\{Path.GetFileNameWithoutExtension(this._filePathOpt)}_*.designtime.log"; + + var logFileDescription = string.Format(ServicesVSResources.IntellisenseBuildFailedDescription, logFilePath); + if (string.IsNullOrWhiteSpace(reason)) + { + return logFileDescription; + } + + return string.Join(Environment.NewLine, logFileDescription, string.Empty, ServicesVSResources.IntellisenseBuildFailedDescriptionExtra, reason); + } + } +} diff --git a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/FileChangeTracker.cs b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/FileChangeTracker.cs index 9104863f3c47925f674029f93480f7a1f6a46237..e491298c72c1fdf46b9d633ad44316bca22c2215 100644 --- a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/FileChangeTracker.cs +++ b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/FileChangeTracker.cs @@ -1,13 +1,9 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Collections.Generic; -using System.Linq; using System.Runtime.InteropServices; -using System.Text; using System.Threading; using System.Threading.Tasks; -using Microsoft.VisualStudio; using Microsoft.VisualStudio.Shell.Interop; using Roslyn.Utilities; @@ -15,6 +11,8 @@ namespace Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem { internal sealed class FileChangeTracker : IVsFileChangeEvents, IDisposable { + private const uint FileChangeFlags = (uint)(_VSFILECHANGEFLAGS.VSFILECHG_Time | _VSFILECHANGEFLAGS.VSFILECHG_Add | _VSFILECHANGEFLAGS.VSFILECHG_Del | _VSFILECHANGEFLAGS.VSFILECHG_Size); + private static readonly Lazy s_none = new Lazy(() => /* value doesn't matter*/ 42424242, LazyThreadSafetyMode.ExecutionAndPublication); private readonly IVsFileChangeEx _fileChangeService; @@ -74,7 +72,7 @@ public void StartFileChangeListeningAsync() { uint newCookie; Marshal.ThrowExceptionForHR( - _fileChangeService.AdviseFileChange(_filePath, (uint)_VSFILECHANGEFLAGS.VSFILECHG_Time, this, out newCookie)); + _fileChangeService.AdviseFileChange(_filePath, FileChangeFlags, this, out newCookie)); return newCookie; }, LazyThreadSafetyMode.ExecutionAndPublication); diff --git a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/IVisualStudioWorkspaceHost2.cs b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/IVisualStudioWorkspaceHost2.cs new file mode 100644 index 0000000000000000000000000000000000000000..6082db16b3c7ba6f26f514a0bedf4ff8db002071 --- /dev/null +++ b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/IVisualStudioWorkspaceHost2.cs @@ -0,0 +1,12 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.CodeAnalysis; + +namespace Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem +{ + // TODO: Find a better name for this + internal interface IVisualStudioWorkspaceHost2 + { + void OnHasAllInformation(ProjectId projectId, bool hasAllInformation); + } +} diff --git a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/Interop/IIntellisenseBuildTarget.cs b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/Interop/IIntellisenseBuildTarget.cs new file mode 100644 index 0000000000000000000000000000000000000000..13355f19a0c5cede9205c67386608f599ee0c84d --- /dev/null +++ b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/Interop/IIntellisenseBuildTarget.cs @@ -0,0 +1,16 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Runtime.InteropServices; + +namespace Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem.Interop +{ + [ComImport] + [InterfaceType(ComInterfaceType.InterfaceIsIUnknown)] + [Guid("F304ABA7-2448-4EE9-A706-D1838F200398")] + internal interface IIntellisenseBuildTarget + { + // currently, reason is not being used. + void SetIntellisenseBuildResult(bool succeeded, [MarshalAs(UnmanagedType.LPWStr)] string reason); + } +} diff --git a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/MiscellaneousFilesWorkspace.HostProject.cs b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/MiscellaneousFilesWorkspace.HostProject.cs index 6f370f083cc26cf67fd05cd7bc20868492fb7e23..a7509a45d49dbd3fb397724010e6a59ae894bdbd 100644 --- a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/MiscellaneousFilesWorkspace.HostProject.cs +++ b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/MiscellaneousFilesWorkspace.HostProject.cs @@ -46,7 +46,7 @@ public HostProject(Workspace workspace, SolutionId solutionId, string languageNa public ProjectInfo CreateProjectInfoForCurrentState() { - return ProjectInfo.Create( + var info = ProjectInfo.Create( this.Id, _version, name: ServicesVSResources.MiscellaneousFiles, @@ -60,6 +60,10 @@ public ProjectInfo CreateProjectInfoForCurrentState() metadataReferences: _metadataReferences, isSubmission: false, hostObjectType: null); + + // misc project will never be fully loaded since, by defintion, it won't know + // what the full set of information is. + return info.WithHasAllInformation(hasAllInformation: false); } public Microsoft.VisualStudio.Shell.Interop.IVsHierarchy Hierarchy => null; diff --git a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProjectTracker_IVsSolutionWorkingFoldersEvents.cs b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProjectTracker_IVsSolutionWorkingFoldersEvents.cs index aeff2f0bf2c1a511e9c645d30a26ebdecad5e04d..eb229b354f58298b5b5242a886d5e067dc1d52df 100644 --- a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProjectTracker_IVsSolutionWorkingFoldersEvents.cs +++ b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProjectTracker_IVsSolutionWorkingFoldersEvents.cs @@ -14,16 +14,7 @@ void IVsSolutionWorkingFoldersEvents.OnAfterLocationChange(uint location, bool c } // notify the working folder change - NotifyWorkspaceHosts(host => - { - var workingFolder = host as IVisualStudioWorkingFolder; - if (workingFolder == null) - { - return; - } - - workingFolder.OnAfterWorkingFolderChange(); - }); + NotifyWorkspaceHosts(host => (host as IVisualStudioWorkingFolder)?.OnAfterWorkingFolderChange()); } void IVsSolutionWorkingFoldersEvents.OnQueryLocationChange(uint location, out bool pfCanMoveContent) @@ -36,16 +27,7 @@ void IVsSolutionWorkingFoldersEvents.OnQueryLocationChange(uint location, out bo // notify the working folder change pfCanMoveContent = true; - NotifyWorkspaceHosts(host => - { - var workingFolder = host as IVisualStudioWorkingFolder; - if (workingFolder == null) - { - return; - } - - workingFolder.OnBeforeWorkingFolderChange(); - }); + NotifyWorkspaceHosts(host => (host as IVisualStudioWorkingFolder)?.OnBeforeWorkingFolderChange()); } } } diff --git a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioWorkspaceImpl.cs b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioWorkspaceImpl.cs index d94ff0099689f429b2e0a60fb61eb7a2d4ae8b72..31ec61e0dc3d78cc8cf9ad8e85649301f240eaf4 100644 --- a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioWorkspaceImpl.cs +++ b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioWorkspaceImpl.cs @@ -1164,7 +1164,7 @@ internal override bool CanAddProjectReference(ProjectId referencingProject, Proj /// A trivial implementation of that just /// forwards the calls down to the underlying Workspace. /// - protected class VisualStudioWorkspaceHost : IVisualStudioWorkspaceHost, IVisualStudioWorkingFolder + protected sealed class VisualStudioWorkspaceHost : IVisualStudioWorkspaceHost, IVisualStudioWorkspaceHost2, IVisualStudioWorkingFolder { private readonly VisualStudioWorkspaceImpl _workspace; @@ -1400,6 +1400,11 @@ void IVisualStudioWorkspaceHost.OnAdditionalDocumentTextUpdatedOnDisk(DocumentId _workspace.OnAdditionalDocumentTextUpdatedOnDisk(id); } + void IVisualStudioWorkspaceHost2.OnHasAllInformation(ProjectId projectId, bool hasAllInformation) + { + _workspace.OnHasAllInformationChanged(projectId, hasAllInformation); + } + void IVisualStudioWorkingFolder.OnBeforeWorkingFolderChange() { UnregisterPrimarySolutionForPersistentStorage(_workspace.CurrentSolution.Id, synchronousShutdown: true); diff --git a/src/VisualStudio/Core/Def/Packaging/IPackageSearchDelayService.cs b/src/VisualStudio/Core/Def/Packaging/IPackageSearchDelayService.cs index f0cf44ef54e8bac15464fc737a91431606e5dcb7..5aeae668d8020fdb0c68be5ba47866b565578530 100644 --- a/src/VisualStudio/Core/Def/Packaging/IPackageSearchDelayService.cs +++ b/src/VisualStudio/Core/Def/Packaging/IPackageSearchDelayService.cs @@ -14,17 +14,27 @@ namespace Microsoft.VisualStudio.LanguageServices.Packaging internal interface IPackageSearchDelayService { /// - /// They time to wait after a successful update (default = 1 day). + /// The time to wait after a successful update (default = 1 day). /// TimeSpan UpdateSucceededDelay { get; } /// - /// They time to wait after a failed update (default = 1 minute). + /// The time to wait after a simple expected sort of failure (i.e. IO exceptions, + /// network exceptions, etc). Things we can recover from and would expect would + /// be transient. /// - TimeSpan UpdateFailedDelay { get; } + TimeSpan ExpectedFailureDelay { get; } /// - /// They time to wait after writing to disk fails (default = 10 seconds). + /// The time to wait after a catastrophic failed update (default = 1 day). For + /// example, if we download the full DB xml from the server and we cannot parse + /// it. Retrying soon after will not help. We'll just have to wait until proper + /// data is on the server for us to query. + /// + TimeSpan CatastrophicFailureDelay { get; } + + /// + /// The time to wait after writing to disk fails (default = 10 seconds). /// TimeSpan FileWriteDelay { get; } @@ -33,4 +43,4 @@ internal interface IPackageSearchDelayService /// TimeSpan CachePollDelay { get; } } -} \ No newline at end of file +} diff --git a/src/VisualStudio/Core/Def/Packaging/PackageSearchService.DelayService.cs b/src/VisualStudio/Core/Def/Packaging/PackageSearchService.DelayService.cs index c0c14a411f6c2ac949e3306ae8fd33b94a2f6ad1..395ae950d1bf4348aa394769fc41b4d2d9e17949 100644 --- a/src/VisualStudio/Core/Def/Packaging/PackageSearchService.DelayService.cs +++ b/src/VisualStudio/Core/Def/Packaging/PackageSearchService.DelayService.cs @@ -10,7 +10,8 @@ private class DelayService : IPackageSearchDelayService { public TimeSpan CachePollDelay { get; } = TimeSpan.FromMinutes(1); public TimeSpan FileWriteDelay { get; } = TimeSpan.FromSeconds(10); - public TimeSpan UpdateFailedDelay { get; } = TimeSpan.FromMinutes(1); + public TimeSpan ExpectedFailureDelay { get; } = TimeSpan.FromMinutes(1); + public TimeSpan CatastrophicFailureDelay { get; } = TimeSpan.FromDays(1); public TimeSpan UpdateSucceededDelay { get; } = TimeSpan.FromDays(1); } } diff --git a/src/VisualStudio/Core/Def/Packaging/PackageSearchService.Update.cs b/src/VisualStudio/Core/Def/Packaging/PackageSearchService.Update.cs index 748693e4d93c41cfd651712b5fc8ddc6d2f0887f..9bbc1e62f16c5a960625c69574fc48853085f5a0 100644 --- a/src/VisualStudio/Core/Def/Packaging/PackageSearchService.Update.cs +++ b/src/VisualStudio/Core/Def/Packaging/PackageSearchService.Update.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.IO; using System.IO.Compression; +using System.Security.Cryptography; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -30,6 +31,7 @@ internal partial class PackageSearchService { // Internal for testing purposes. internal const string ContentAttributeName = "content"; + internal const string ChecksumAttributeName = "checksum"; internal const string UpToDateAttributeName = "upToDate"; internal const string TooOldAttributeName = "tooOld"; internal const string NugetOrgSource = "nuget.org"; @@ -220,7 +222,7 @@ private async Task UpdateDatabaseInBackgroundWorkerAsync() // Note: we skip OperationCanceledException because it's not 'bad'. // It's the standard way to indicate that we've been asked to shut // down. - var delay = _service._delayService.UpdateFailedDelay; + var delay = _service._delayService.ExpectedFailureDelay; _service.LogException(e, $"Error occurred updating. Retrying update in {delay}"); return delay; } @@ -261,11 +263,35 @@ private async Task ProcessFullDatabaseXElementAsync(XElement element) _service.LogInfo("Processing full database element"); // Convert the database contents in the xml to a byte[]. - var bytes = ParseDatabaseElement(element); + byte[] bytes; + if (!TryParseDatabaseElement(element, out bytes)) + { + // Something was wrong with the full database. Trying again soon after won't + // really help. We'll just get the same busted XML from the remote service + // cache. So we want to actually wait some long enough amount of time so that + // we can retrieve good data the next time around. + + var failureDelay = _service._delayService.CatastrophicFailureDelay; + _service.LogInfo($"Unable to parse full database element. Update again in {failureDelay}"); + return failureDelay; + } // Make a database out of that and set it to our in memory database that we'll be // searching. - CreateAndSetInMemoryDatabase(bytes); + try + { + CreateAndSetInMemoryDatabase(bytes); + } + catch (Exception e) when (_service._reportAndSwallowException(e)) + { + // We retrieved bytes from the server, but we couldn't make a DB + // out of it. That's very bad. Just trying again one minute later + // isn't going to help. We need to wait until there is good data + // on the server for us to download. + var failureDelay = _service._delayService.CatastrophicFailureDelay; + _service.LogInfo($"Unable to create database from full database element. Update again in {failureDelay}"); + return failureDelay; + } // Write the file out to disk so we'll have it the next time we launch VS. Do this // after we set the in-memory instance so we at least have something to search while @@ -578,21 +604,53 @@ private async Task RepeatIOAsync(Action action) } } - private byte[] ParseDatabaseElement(XElement element) + private bool TryParseDatabaseElement(XElement element, out byte[] bytes) { _service.LogInfo("Parsing database element"); var contentsAttribute = element.Attribute(ContentAttributeName); if (contentsAttribute == null) { - throw new FormatException($"Database element invalid. Missing '{ContentAttributeName}' attribute"); + _service._reportAndSwallowException( + new FormatException($"Database element invalid. Missing '{ContentAttributeName}' attribute")); + + bytes = null; + return false; + } + + var contentBytes = ConvertContentAttribute(contentsAttribute); + + var checksumAttribute = element.Attribute(ChecksumAttributeName); + if (checksumAttribute != null) + { + var expectedChecksum = checksumAttribute.Value; + string actualChecksum; + using (var sha256 = SHA256.Create()) + { + actualChecksum = Convert.ToBase64String(sha256.ComputeHash(contentBytes)); + } + + if (!StringComparer.Ordinal.Equals(expectedChecksum, actualChecksum)) + { + _service._reportAndSwallowException( + new FormatException($"Checksum mismatch: expected != actual. {expectedChecksum} != {actualChecksum}")); + + bytes = null; + return false; + } } + bytes = contentBytes; + return true; + } + + private byte[] ConvertContentAttribute(XAttribute contentsAttribute) + { var text = contentsAttribute.Value; var compressedBytes = Convert.FromBase64String(text); - using (var inStream = new MemoryStream(compressedBytes)) using (var outStream = new MemoryStream()) { + using (var inStream = new MemoryStream(compressedBytes)) using (var deflateStream = new DeflateStream(inStream, CompressionMode.Decompress)) { deflateStream.CopyTo(outStream); diff --git a/src/VisualStudio/Core/Def/RoslynPackage.cs b/src/VisualStudio/Core/Def/RoslynPackage.cs index 3276de69f39f76b6275017406399a01ba6e30700..68584f456f6aceaab0414cb0e04c44454a1c9eba 100644 --- a/src/VisualStudio/Core/Def/RoslynPackage.cs +++ b/src/VisualStudio/Core/Def/RoslynPackage.cs @@ -223,6 +223,7 @@ protected override void Dispose(bool disposing) private void ReportSessionWideTelemetry() { PersistedVersionStampLogger.LogSummary(); + LinkedFileDiffMergingLogger.ReportTelemetry(); } private void RegisterFindResultsLibraryManager() diff --git a/src/VisualStudio/Core/Def/ServicesVSResources.Designer.cs b/src/VisualStudio/Core/Def/ServicesVSResources.Designer.cs index 62397e2ad9b53557ede0b7bbf7f05e0c475ad50c..c80fbf4234f48169e8aa5eea8e5010ff020d9479 100644 --- a/src/VisualStudio/Core/Def/ServicesVSResources.Designer.cs +++ b/src/VisualStudio/Core/Def/ServicesVSResources.Designer.cs @@ -690,6 +690,50 @@ internal class ServicesVSResources { } } + /// + /// Looks up a localized string similar to To see what caused the issue, please try below. + /// + ///1. Close Visual Studio + ///2. Open a Visual Studio Developer Command Prompt + ///3. Set environment variable “TraceDesignTime” to true (set TraceDesignTime=true) + ///4. Delete .vs directory/.suo file + ///5. Restart VS from the command prompt you set the environment varaible (devenv) + ///6. Open the solution + ///7. Check '{0}' and look for the failed tasks (FAILED). + /// + internal static string IntellisenseBuildFailedDescription { + get { + return ResourceManager.GetString("IntellisenseBuildFailedDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Additional information:. + /// + internal static string IntellisenseBuildFailedDescriptionExtra { + get { + return ResourceManager.GetString("IntellisenseBuildFailedDescriptionExtra", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Error encountered while loading the project. Some project features, such as full solution analysis for the failed project and projects that depend on it, have been disabled.. + /// + internal static string IntellisenseBuildFailedMessage { + get { + return ResourceManager.GetString("IntellisenseBuildFailedMessage", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Project loading failed.. + /// + internal static string IntellisenseBuildFailedTitle { + get { + return ResourceManager.GetString("IntellisenseBuildFailedTitle", resourceCulture); + } + } + /// /// Looks up a localized string similar to Installing '{0}' failed. /// diff --git a/src/VisualStudio/Core/Def/ServicesVSResources.resx b/src/VisualStudio/Core/Def/ServicesVSResources.resx index d54662ac7cfa17c16448add54aa33145c8f5f8a8..a68620bdfad5a0bf6e646c3c23f45e4a3c6ec937 100644 --- a/src/VisualStudio/Core/Def/ServicesVSResources.resx +++ b/src/VisualStudio/Core/Def/ServicesVSResources.resx @@ -641,6 +641,26 @@ Use the dropdown to view and switch to other projects this file may belong to. Package uninstall failed: {0} + + Error encountered while loading the project. Some project features, such as full solution analysis for the failed project and projects that depend on it, have been disabled. + + + Project loading failed. + + + To see what caused the issue, please try below. + +1. Close Visual Studio +2. Open a Visual Studio Developer Command Prompt +3. Set environment variable “TraceDesignTime” to true (set TraceDesignTime=true) +4. Delete .vs directory/.suo file +5. Restart VS from the command prompt you set the environment varaible (devenv) +6. Open the solution +7. Check '{0}' and look for the failed tasks (FAILED) + + + Additional information: + Installing '{0}' failed. diff --git a/src/VisualStudio/Core/Def/ServicesVisualStudio.csproj b/src/VisualStudio/Core/Def/ServicesVisualStudio.csproj index a94f518ac640bf4a1867a72558314c760a68e147..ddd3d9b26a1ec91cd131f75e425347e5a5f1cd7b 100644 --- a/src/VisualStudio/Core/Def/ServicesVisualStudio.csproj +++ b/src/VisualStudio/Core/Def/ServicesVisualStudio.csproj @@ -85,9 +85,12 @@ + + + diff --git a/src/VisualStudio/Core/Test/Packaging/PackageSearchServiceTests.vb b/src/VisualStudio/Core/Test/Packaging/PackageSearchServiceTests.vb index 1b983d1da2a8bba0a62938da2390cdca06f9233b..1802dd9602ad09cbed6b14126c45e5dd8d7544c6 100644 --- a/src/VisualStudio/Core/Test/Packaging/PackageSearchServiceTests.vb +++ b/src/VisualStudio/Core/Test/Packaging/PackageSearchServiceTests.vb @@ -119,6 +119,50 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.Packaging clientMock.Verify() End Function + + Public Async Function FailureToParseFullDBAtXmlLevelTakesCatastrophicPath() As Task + Dim cancellationTokenSource = New CancellationTokenSource() + + Dim ioMock = New Mock(Of IPackageSearchIOService)() + + ' Simlute the local database being missing. + ioMock.Setup(Function(s) s.Exists(It.IsAny(Of FileSystemInfo))).Returns(False) + + Dim clientMock = CreateClientMock(CreateStream(New XElement("Database", + New XAttribute(PackageSearchService.ContentAttributeName, ""), + New XAttribute(PackageSearchService.ChecksumAttributeName, Convert.ToBase64String(New Byte() {0, 1, 2}))))) + + Dim serviceMock = New Mock(Of IPackageSearchRemoteControlService)(MockBehavior.Strict) + + ' The client should request the 'Latest' database from the server. + ' Cancel processing at that point so the test can complete. + serviceMock.Setup( + Function(s) s.CreateClient(It.IsAny(Of String), It.IsRegex(".*Latest.*"), It.IsAny(Of Integer))). + Returns(clientMock.Object) + + Dim delayMock = New Mock(Of IPackageSearchDelayService)(MockBehavior.Strict) + delayMock.SetupGet(Function(s) s.CatastrophicFailureDelay).Returns(TimeSpan.Zero).Callback( + AddressOf cancellationTokenSource.Cancel) + + Dim searchService = New PackageSearchService( + installerService:=TestInstallerService.Instance, + remoteControlService:=serviceMock.Object, + logService:=TestLogService.Instance, + delayService:=delayMock.Object, + ioService:=ioMock.Object, + patchService:=Nothing, + databaseFactoryService:=Nothing, + localSettingsDirectory:="TestDirectory", + reportAndSwallowException:=s_allButMoqExceptions, + cancellationTokenSource:=cancellationTokenSource) + + Await searchService.UpdateSourceInBackgroundAsync(PackageSearchService.NugetOrgSource) + ioMock.Verify() + serviceMock.Verify() + clientMock.Verify() + delayMock.Verify() + End Function + Public Async Function TestClientDisposedAfterUse() As Task Dim cancellationTokenSource = New CancellationTokenSource() @@ -180,7 +224,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.Packaging ' control when we do our next loop. ' Cancel processing at that point so the test can complete. Dim delayMock = New Mock(Of IPackageSearchDelayService)(MockBehavior.Strict) - delayMock.SetupGet(Function(s) s.UpdateFailedDelay).Returns(TimeSpan.Zero).Callback( + delayMock.SetupGet(Function(s) s.ExpectedFailureDelay).Returns(TimeSpan.Zero).Callback( AddressOf cancellationTokenSource.Cancel) Dim searchService = New PackageSearchService( @@ -203,7 +247,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.Packaging End Function - Public Async Function FailureToParseDBRunsFailureLoopPath() As Task + Public Async Function FailureToParseFullDBAtElfieLevelTakesCatastrophicPath() As Task Dim cancellationTokenSource = New CancellationTokenSource() Dim ioMock = New Mock(Of IPackageSearchIOService)() @@ -223,7 +267,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.Packaging ' control when we do our next loop. ' Cancel processing at that point so the test can complete. Dim delayMock = New Mock(Of IPackageSearchDelayService)(MockBehavior.Strict) - delayMock.SetupGet(Function(s) s.UpdateFailedDelay).Returns(TimeSpan.Zero).Callback( + delayMock.SetupGet(Function(s) s.CatastrophicFailureDelay).Returns(TimeSpan.Zero).Callback( AddressOf cancellationTokenSource.Cancel) Dim searchService = New PackageSearchService( @@ -667,7 +711,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.Packaging End Get End Property - Public ReadOnly Property UpdateFailedDelay As TimeSpan Implements IPackageSearchDelayService.UpdateFailedDelay + Public ReadOnly Property ExpectedFailureDelay As TimeSpan Implements IPackageSearchDelayService.ExpectedFailureDelay Get Return TimeSpan.Zero End Get @@ -678,6 +722,12 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.Packaging Return TimeSpan.Zero End Get End Property + + Public ReadOnly Property CatastrophicFailureDelay As TimeSpan Implements IPackageSearchDelayService.CatastrophicFailureDelay + Get + Return TimeSpan.Zero + End Get + End Property End Class Private Class TestInstallerService diff --git a/src/VisualStudio/Core/Test/ProjectSystemShim/Framework/MockVsFileChangeEx.vb b/src/VisualStudio/Core/Test/ProjectSystemShim/Framework/MockVsFileChangeEx.vb index 0d8dfaf6da4d2e8ba772703c8d82e0eaedade085..a699c85fdb6d65f0203759e9f78a7de2429a7f22 100644 --- a/src/VisualStudio/Core/Test/ProjectSystemShim/Framework/MockVsFileChangeEx.vb +++ b/src/VisualStudio/Core/Test/ProjectSystemShim/Framework/MockVsFileChangeEx.vb @@ -15,7 +15,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Fr End Function Public Function AdviseFileChange(pszMkDocument As String, grfFilter As UInteger, pFCE As IVsFileChangeEvents, ByRef pvsCookie As UInteger) As Integer Implements IVsFileChangeEx.AdviseFileChange - If grfFilter <> _VSFILECHANGEFLAGS.VSFILECHG_Time Then + If (grfFilter And _VSFILECHANGEFLAGS.VSFILECHG_Time) <> _VSFILECHANGEFLAGS.VSFILECHG_Time Then Throw New NotImplementedException() End If diff --git a/src/VisualStudio/VisualBasic/Impl/ProjectSystemShim/VisualBasicProject.vb b/src/VisualStudio/VisualBasic/Impl/ProjectSystemShim/VisualBasicProject.vb index 4fc7923cf2ae28f6ab648c90763c18ad49313a84..55eb09c2f12fb59764214d42a3d1619f12b9fbef 100644 --- a/src/VisualStudio/VisualBasic/Impl/ProjectSystemShim/VisualBasicProject.vb +++ b/src/VisualStudio/VisualBasic/Impl/ProjectSystemShim/VisualBasicProject.vb @@ -58,7 +58,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.VisualBasic.ProjectSystemShim Public Function AddEmbeddedMetaDataReference(wszFileName As String) As Integer Implements IVbCompilerProject.AddEmbeddedMetaDataReference Try - Return AddMetadataReferenceAndTryConvertingToProjectReferenceIfPossible(wszFileName, New MetadataReferenceProperties(embedInteropTypes:=True), VSConstants.S_FALSE) + Return AddMetadataReferenceAndTryConvertingToProjectReferenceIfPossible(wszFileName, New MetadataReferenceProperties(embedInteropTypes:=True)) Catch e As Exception When FilterException(e) Throw ExceptionUtilities.Unreachable End Try @@ -72,7 +72,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.VisualBasic.ProjectSystemShim Return VSConstants.S_OK End If - Return AddMetadataReferenceAndTryConvertingToProjectReferenceIfPossible(wszFileName, New MetadataReferenceProperties(), VSConstants.S_FALSE) + Return AddMetadataReferenceAndTryConvertingToProjectReferenceIfPossible(wszFileName, New MetadataReferenceProperties()) Catch e As Exception When FilterException(e) Throw ExceptionUtilities.Unreachable End Try @@ -414,7 +414,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.VisualBasic.ProjectSystemShim If HasMetadataReference(newRuntimeLibrary) Then _explicitlyAddedDefaultReferences.Add(newRuntimeLibrary) Else - MyBase.AddMetadataReferenceAndTryConvertingToProjectReferenceIfPossible(newRuntimeLibrary, MetadataReferenceProperties.Assembly, hResultForMissingFile:=0) + MyBase.AddMetadataReferenceAndTryConvertingToProjectReferenceIfPossible(newRuntimeLibrary, MetadataReferenceProperties.Assembly) End If Next End If diff --git a/src/Workspaces/Core/Portable/Diagnostics/Extensions.cs b/src/Workspaces/Core/Portable/Diagnostics/Extensions.cs index 70dee2bb2aab61e6782ef501d4bcc8193c48e99a..1f63c1e7cc2f642f6651cdef8bb186892328ada2 100644 --- a/src/Workspaces/Core/Portable/Diagnostics/Extensions.cs +++ b/src/Workspaces/Core/Portable/Diagnostics/Extensions.cs @@ -14,7 +14,7 @@ namespace Microsoft.CodeAnalysis.Diagnostics { internal static class Extensions { - private static readonly CultureInfo s_USCultureInfo = new CultureInfo("en-US"); + public static readonly CultureInfo s_USCultureInfo = new CultureInfo("en-US"); public static string GetBingHelpMessage(this Diagnostic diagnostic, Workspace workspace = null) { diff --git a/src/Workspaces/Core/Portable/Formatting/Rules/BaseIndentationFormattingRule.cs b/src/Workspaces/Core/Portable/Formatting/Rules/BaseIndentationFormattingRule.cs index 04f498c5d9626ed891c6cc7bcd9dc0a356af0c22..f441fbfc6b13268be15c78f662d2c4f1d4262710 100644 --- a/src/Workspaces/Core/Portable/Formatting/Rules/BaseIndentationFormattingRule.cs +++ b/src/Workspaces/Core/Portable/Formatting/Rules/BaseIndentationFormattingRule.cs @@ -203,6 +203,17 @@ private static TextSpan GetSpanFromTokens(TextSpan span, SyntaxToken token1, Syn } } + if (token1.Equals(token2) && end < start) + { + // This can happen if `token1.Span` is larger than `span` on each end (due to trivia) and occurs when + // only a single token is projected into a buffer and the projection is sandwiched between two other + // projections into the same backing buffer. An example of this is during broken code scenarios when + // typing certain Razor `@` directives. + var temp = end; + end = start; + start = temp; + } + return TextSpan.FromBounds(start, end); } } diff --git a/src/Workspaces/Core/Portable/LinkedFileDiffMerging/LinkedFileDiffMergingLogger.cs b/src/Workspaces/Core/Portable/LinkedFileDiffMerging/LinkedFileDiffMergingLogger.cs new file mode 100644 index 0000000000000000000000000000000000000000..0dbd5963b16f839857adcd70f582d29a8e6cb9d7 --- /dev/null +++ b/src/Workspaces/Core/Portable/LinkedFileDiffMerging/LinkedFileDiffMergingLogger.cs @@ -0,0 +1,121 @@ +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Linq; +using System.Threading; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Internal.Log; +using static Microsoft.CodeAnalysis.LinkedFileDiffMergingSession; + +namespace Microsoft.CodeAnalysis +{ + internal class LinkedFileDiffMergingLogger + { + private static LogAggregator LogAggregator = new LogAggregator(); + + internal enum MergeInfo + { + SessionsWithLinkedFiles, + LinkedFileGroupsProcessed, + IdenticalDiffs, + IsolatedDiffs, + OverlappingDistinctDiffs, + OverlappingDistinctDiffsWithSameSpan, + OverlappingDistinctDiffsWithSameSpanAndSubstringRelation, + InsertedMergeConflictComments, + InsertedMergeConflictCommentsAtAdjustedLocation + } + + internal static void LogSession(Workspace workspace, LinkedFileDiffMergingSessionInfo sessionInfo) + { + if (sessionInfo.LinkedFileGroups.Count > 1) + { + LogNewSessionWithLinkedFiles(); + LogNumberOfLinkedFileGroupsProcessed(sessionInfo.LinkedFileGroups.Count); + + foreach (var groupInfo in sessionInfo.LinkedFileGroups) + { + LogNumberOfIdenticalDiffs(groupInfo.IdenticalDiffs); + LogNumberOfIsolatedDiffs(groupInfo.IsolatedDiffs); + LogNumberOfOverlappingDistinctDiffs(groupInfo.OverlappingDistinctDiffs); + LogNumberOfOverlappingDistinctDiffsWithSameSpan(groupInfo.OverlappingDistinctDiffsWithSameSpan); + LogNumberOfOverlappingDistinctDiffsWithSameSpanAndSubstringRelation(groupInfo.OverlappingDistinctDiffsWithSameSpanAndSubstringRelation); + LogNumberOfInsertedMergeConflictComments(groupInfo.InsertedMergeConflictComments); + LogNumberOfInsertedMergeConflictCommentsAtAdjustedLocation(groupInfo.InsertedMergeConflictCommentsAtAdjustedLocation); + + if (groupInfo.InsertedMergeConflictComments > 0 || + groupInfo.InsertedMergeConflictCommentsAtAdjustedLocation > 0) + { + Logger.Log(FunctionId.Workspace_Solution_LinkedFileDiffMergingSession_LinkedFileGroup, SessionLogMessage.Create(groupInfo)); + } + } + } + } + + private static void LogNewSessionWithLinkedFiles() => + Log((int)MergeInfo.SessionsWithLinkedFiles, 1); + + private static void LogNumberOfLinkedFileGroupsProcessed(int linkedFileGroupsProcessed) => + Log((int)MergeInfo.LinkedFileGroupsProcessed, linkedFileGroupsProcessed); + + private static void LogNumberOfIdenticalDiffs(int identicalDiffs) => + Log((int)MergeInfo.IdenticalDiffs, identicalDiffs); + + private static void LogNumberOfIsolatedDiffs(int isolatedDiffs) => + Log((int)MergeInfo.IsolatedDiffs, isolatedDiffs); + + private static void LogNumberOfOverlappingDistinctDiffs(int overlappingDistinctDiffs) => + Log((int)MergeInfo.OverlappingDistinctDiffs, overlappingDistinctDiffs); + + private static void LogNumberOfOverlappingDistinctDiffsWithSameSpan(int overlappingDistinctDiffsWithSameSpan) => + Log((int)MergeInfo.OverlappingDistinctDiffsWithSameSpan, overlappingDistinctDiffsWithSameSpan); + + private static void LogNumberOfOverlappingDistinctDiffsWithSameSpanAndSubstringRelation(int overlappingDistinctDiffsWithSameSpanAndSubstringRelation) => + Log((int)MergeInfo.OverlappingDistinctDiffsWithSameSpanAndSubstringRelation, overlappingDistinctDiffsWithSameSpanAndSubstringRelation); + + private static void LogNumberOfInsertedMergeConflictComments(int insertedMergeConflictComments) => + Log((int)MergeInfo.InsertedMergeConflictComments, insertedMergeConflictComments); + + private static void LogNumberOfInsertedMergeConflictCommentsAtAdjustedLocation(int insertedMergeConflictCommentsAtAdjustedLocation) => + Log((int)MergeInfo.InsertedMergeConflictCommentsAtAdjustedLocation, insertedMergeConflictCommentsAtAdjustedLocation); + + private static void Log(int mergeInfo, int count) + { + LogAggregator.IncreaseCountBy(mergeInfo, count); + } + + internal static void ReportTelemetry() + { + Logger.Log(FunctionId.Workspace_Solution_LinkedFileDiffMergingSession, KeyValueLogMessage.Create(m => + { + foreach (var kv in LogAggregator) + { + var mergeInfo = ((MergeInfo)kv.Key).ToString("f"); + m[mergeInfo] = kv.Value.GetCount(); + } + })); + } + + private static class SessionLogMessage + { + private const string LinkedDocuments = nameof(LinkedDocuments); + private const string DocumentsWithChanges = nameof(DocumentsWithChanges); + + public static KeyValueLogMessage Create(LinkedFileGroupSessionInfo groupInfo) + { + return KeyValueLogMessage.Create(m => + { + m[LinkedDocuments] = groupInfo.LinkedDocuments; + m[DocumentsWithChanges] = groupInfo.DocumentsWithChanges; + m[MergeInfo.IdenticalDiffs.ToString("f")] = groupInfo.IdenticalDiffs; + m[MergeInfo.IsolatedDiffs.ToString("f")] = groupInfo.IsolatedDiffs; + m[MergeInfo.OverlappingDistinctDiffs.ToString("f")] = groupInfo.OverlappingDistinctDiffs; + m[MergeInfo.OverlappingDistinctDiffsWithSameSpan.ToString("f")] = groupInfo.OverlappingDistinctDiffsWithSameSpan; + m[MergeInfo.OverlappingDistinctDiffsWithSameSpanAndSubstringRelation.ToString("f")] = groupInfo.OverlappingDistinctDiffsWithSameSpanAndSubstringRelation; + m[MergeInfo.InsertedMergeConflictComments.ToString("f")] = groupInfo.InsertedMergeConflictComments; + m[MergeInfo.InsertedMergeConflictCommentsAtAdjustedLocation.ToString("f")] = groupInfo.InsertedMergeConflictCommentsAtAdjustedLocation; + }); + } + } + } +} diff --git a/src/Workspaces/Core/Portable/LinkedFileDiffMerging/LinkedFileDiffMergingSession.cs b/src/Workspaces/Core/Portable/LinkedFileDiffMerging/LinkedFileDiffMergingSession.cs index aae27cbaedf71a4c0fca6334445cb10069979806..595f0b337437524a665ebf1e8fc208a0c24fb8af 100644 --- a/src/Workspaces/Core/Portable/LinkedFileDiffMerging/LinkedFileDiffMergingSession.cs +++ b/src/Workspaces/Core/Portable/LinkedFileDiffMerging/LinkedFileDiffMergingSession.cs @@ -316,68 +316,12 @@ private IEnumerable NormalizeChanges(IEnumerable changes private void LogLinkedFileDiffMergingSessionInfo(LinkedFileDiffMergingSessionInfo sessionInfo) { - // don't report telemetry if (!_logSessionInfo) { return; } - var sessionId = SessionLogMessage.GetNextId(); - - Logger.Log(FunctionId.Workspace_Solution_LinkedFileDiffMergingSession, SessionLogMessage.Create(sessionId, sessionInfo)); - - foreach (var groupInfo in sessionInfo.LinkedFileGroups) - { - Logger.Log(FunctionId.Workspace_Solution_LinkedFileDiffMergingSession_LinkedFileGroup, SessionLogMessage.Create(sessionId, groupInfo)); - } - } - - internal static class SessionLogMessage - { - private const string SessionId = nameof(SessionId); - private const string HasLinkedFile = nameof(HasLinkedFile); - - private const string LinkedDocuments = nameof(LinkedDocuments); - private const string DocumentsWithChanges = nameof(DocumentsWithChanges); - private const string IdenticalDiffs = nameof(IdenticalDiffs); - private const string IsolatedDiffs = nameof(IsolatedDiffs); - private const string OverlappingDistinctDiffs = nameof(OverlappingDistinctDiffs); - private const string OverlappingDistinctDiffsWithSameSpan = nameof(OverlappingDistinctDiffsWithSameSpan); - private const string OverlappingDistinctDiffsWithSameSpanAndSubstringRelation = nameof(OverlappingDistinctDiffsWithSameSpanAndSubstringRelation); - private const string InsertedMergeConflictComments = nameof(InsertedMergeConflictComments); - private const string InsertedMergeConflictCommentsAtAdjustedLocation = nameof(InsertedMergeConflictCommentsAtAdjustedLocation); - - public static KeyValueLogMessage Create(int sessionId, LinkedFileDiffMergingSessionInfo sessionInfo) - { - return KeyValueLogMessage.Create(m => - { - m[SessionId] = sessionId; - m[HasLinkedFile] = sessionInfo.LinkedFileGroups.Count > 0; - }); - } - - public static KeyValueLogMessage Create(int sessionId, LinkedFileGroupSessionInfo groupInfo) - { - return KeyValueLogMessage.Create(m => - { - m[SessionId] = sessionId; - - m[LinkedDocuments] = groupInfo.LinkedDocuments; - m[DocumentsWithChanges] = groupInfo.DocumentsWithChanges; - m[IdenticalDiffs] = groupInfo.IdenticalDiffs; - m[IsolatedDiffs] = groupInfo.IsolatedDiffs; - m[OverlappingDistinctDiffs] = groupInfo.OverlappingDistinctDiffs; - m[OverlappingDistinctDiffsWithSameSpan] = groupInfo.OverlappingDistinctDiffsWithSameSpan; - m[OverlappingDistinctDiffsWithSameSpanAndSubstringRelation] = groupInfo.OverlappingDistinctDiffsWithSameSpanAndSubstringRelation; - m[InsertedMergeConflictComments] = groupInfo.InsertedMergeConflictComments; - m[InsertedMergeConflictCommentsAtAdjustedLocation] = groupInfo.InsertedMergeConflictCommentsAtAdjustedLocation; - }); - } - - public static int GetNextId() - { - return LogAggregator.GetNextId(); - } + LinkedFileDiffMergingLogger.LogSession(this._newSolution.Workspace, sessionInfo); } internal class LinkedFileDiffMergingSessionInfo diff --git a/src/Workspaces/Core/Portable/Log/FunctionId.cs b/src/Workspaces/Core/Portable/Log/FunctionId.cs index 5fdcff96caed92d74955ea94cc1cc5374722e4d0..d01a1b70a4fe3746dae750c32fcc9cc947db1dec 100644 --- a/src/Workspaces/Core/Portable/Log/FunctionId.cs +++ b/src/Workspaces/Core/Portable/Log/FunctionId.cs @@ -60,6 +60,7 @@ internal enum FunctionId CommandHandler_GetCommandState, CommandHandler_ExecuteHandlers, + CommandHandler_FormatCommand, Workspace_SourceText_GetChangeRanges, Workspace_Recoverable_RecoverRootAsync, @@ -324,5 +325,6 @@ internal enum FunctionId SymbolTreeInfo_ExceptionInCacheRead, SpellChecker_ExceptionInCacheRead, BKTree_ExceptionInCacheRead, + IntellisenseBuild_Failed, } } diff --git a/src/Workspaces/Core/Portable/Log/LogAggregator.cs b/src/Workspaces/Core/Portable/Log/LogAggregator.cs index 8bf251430e429f444255e07f44081257900c323b..66058e8eea8e593afd59519e1034ad9661ce460c 100644 --- a/src/Workspaces/Core/Portable/Log/LogAggregator.cs +++ b/src/Workspaces/Core/Portable/Log/LogAggregator.cs @@ -64,6 +64,12 @@ public void IncreaseCount(object key) counter.IncreaseCount(); } + public void IncreaseCountBy(object key, int value) + { + var counter = GetCounter(key); + counter.IncreaseCountBy(value); + } + public int GetCount(object key) { Counter counter; @@ -111,6 +117,13 @@ public void IncreaseCount() Interlocked.Increment(ref _count); } + public void IncreaseCountBy(int value) + { + // Counter class probably not needed. but it is here for 2 reasons. + // make handling concurrency easier and be a place holder for different type of counter + Interlocked.Add(ref _count, value); + } + public int GetCount() { return _count; diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/Project.cs b/src/Workspaces/Core/Portable/Workspace/Solution/Project.cs index 8a5cae93519f9b5e945318c588cbe3b30ed64374..9750d0064ff3afb22aaeac919ad0443835f3dd28 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/Project.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/Project.cs @@ -35,55 +35,26 @@ internal Project(Solution solution, ProjectState projectState) _projectState = projectState; } - internal ProjectState State - { - get { return _projectState; } - } - /// /// The solution this project is part of. /// - public Solution Solution - { - get - { - return _solution; - } - } + public Solution Solution => _solution; /// /// The ID of the project. Multiple instances may share the same ID. However, only /// one project may have this ID in any given solution. /// - public ProjectId Id - { - get - { - return _projectState.Id; - } - } + public ProjectId Id => _projectState.Id; /// /// The path to the project file or null if there is no project file. /// - public string FilePath - { - get - { - return _projectState.FilePath; - } - } + public string FilePath => _projectState.FilePath; /// /// The path to the output file, or null if it is not known. /// - public string OutputFilePath - { - get - { - return _projectState.OutputFilePath; - } - } + public string OutputFilePath => _projectState.OutputFilePath; /// /// true if this supports providing data through the @@ -91,195 +62,93 @@ public string OutputFilePath /// /// If false then this method will return null instead. /// - public bool SupportsCompilation - { - get - { - return this.LanguageServices.GetService() != null; - } - } + public bool SupportsCompilation => this.LanguageServices.GetService() != null; /// /// The language services from the host environment associated with this project's language. /// - public HostLanguageServices LanguageServices - { - get { return _projectState.LanguageServices; } - } + public HostLanguageServices LanguageServices => _projectState.LanguageServices; /// /// The language associated with the project. /// - public string Language - { - get - { - return _projectState.LanguageServices.Language; - } - } + public string Language => _projectState.LanguageServices.Language; /// /// The name of the assembly this project represents. /// - public string AssemblyName - { - get - { - return _projectState.AssemblyName; - } - } + public string AssemblyName => _projectState.AssemblyName; /// /// The name of the project. This may be different than the assembly name. /// - public string Name - { - get - { - return _projectState.Name; - } - } + public string Name => _projectState.Name; /// /// The list of all other metadata sources (assemblies) that this project references. /// - public IReadOnlyList MetadataReferences - { - get - { - return _projectState.MetadataReferences; - } - } + public IReadOnlyList MetadataReferences => _projectState.MetadataReferences; /// /// The list of all other projects within the same solution that this project references. /// - public IEnumerable ProjectReferences - { - get - { - return _projectState.ProjectReferences.Where(pr => this.Solution.ContainsProject(pr.ProjectId)); - } - } + public IEnumerable ProjectReferences => _projectState.ProjectReferences.Where(pr => this.Solution.ContainsProject(pr.ProjectId)); /// /// The list of all other projects that this project references, including projects that /// are not part of the solution. /// - public IReadOnlyList AllProjectReferences - { - get - { - return _projectState.ProjectReferences; - } - } + public IReadOnlyList AllProjectReferences => _projectState.ProjectReferences; /// /// The list of all the diagnostic analyzer references for this project. /// - public IReadOnlyList AnalyzerReferences - { - get - { - return _projectState.AnalyzerReferences; - } - } + public IReadOnlyList AnalyzerReferences => _projectState.AnalyzerReferences; /// /// The options used by analyzers for this project. /// - public AnalyzerOptions AnalyzerOptions - { - get - { - return _projectState.AnalyzerOptions; - } - } + public AnalyzerOptions AnalyzerOptions => _projectState.AnalyzerOptions; /// /// The options used when building the compilation for this project. /// - public CompilationOptions CompilationOptions - { - get - { - return _projectState.CompilationOptions; - } - } + public CompilationOptions CompilationOptions => _projectState.CompilationOptions; /// /// The options used when parsing documents for this project. /// - public ParseOptions ParseOptions - { - get - { - return _projectState.ParseOptions; - } - } + public ParseOptions ParseOptions => _projectState.ParseOptions; /// /// Returns true if this is a submission project. /// - public bool IsSubmission - { - get - { - return _projectState.IsSubmission; - } - } + public bool IsSubmission => _projectState.IsSubmission; /// /// True if the project has any documents. /// - public bool HasDocuments - { - get - { - return _projectState.HasDocuments; - } - } + public bool HasDocuments => _projectState.HasDocuments; /// /// All the document IDs associated with this project. /// - public IReadOnlyList DocumentIds - { - get - { - return _projectState.DocumentIds; - } - } + public IReadOnlyList DocumentIds => _projectState.DocumentIds; /// /// All the additional document IDs associated with this project. /// - public IReadOnlyList AdditionalDocumentIds - { - get - { - return _projectState.AdditionalDocumentIds; - } - } + public IReadOnlyList AdditionalDocumentIds => _projectState.AdditionalDocumentIds; /// /// All the documents associated with this project. /// - public IEnumerable Documents - { - get - { - return _projectState.DocumentIds.Select(GetDocument); - } - } + public IEnumerable Documents => _projectState.DocumentIds.Select(GetDocument); - public IEnumerable AdditionalDocuments - { - get - { - return _projectState.AdditionalDocumentIds.Select(GetAdditionalDocument); - } - } + /// + /// All the additional documents associated with this project. + /// + public IEnumerable AdditionalDocuments => _projectState.AdditionalDocumentIds.Select(GetAdditionalDocument); /// /// True if the project contains a document with the specified ID. @@ -390,12 +259,12 @@ public Task GetCompilationAsync(CancellationToken cancellationToken } /// - /// Determines if the compilation returned by has all the references it's expected to have. + /// Determines if the compilation returned by and all its referenced compilaton are from fully loaded projects. /// // TODO: make this public - internal Task HasCompleteReferencesAsync(CancellationToken cancellationToken = default(CancellationToken)) + internal Task HasSuccessfullyLoadedAsync(CancellationToken cancellationToken = default(CancellationToken)) { - return _solution.HasCompleteReferencesAsync(this, cancellationToken); + return _solution.HasSuccessfullyLoadedAsync(this, cancellationToken); } /// diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/ProjectInfo.cs b/src/Workspaces/Core/Portable/Workspace/Solution/ProjectInfo.cs index de4698824f555e243d2b783537f3b11767222e3b..bb06933ee57195bceb40ad2b38d10719534398a0 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/ProjectInfo.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/ProjectInfo.cs @@ -2,8 +2,6 @@ using System; using System.Collections.Generic; -using System.Collections.Immutable; -using System.Linq; using Microsoft.CodeAnalysis.Diagnostics; using Roslyn.Utilities; using System.Diagnostics; @@ -96,6 +94,13 @@ public sealed class ProjectInfo /// public Type HostObjectType { get; } + /// + /// True if project information is complete. In some workspace hosts, it is possible + /// a project only has partial information. In such cases, a project might not have all + /// information on its files or references. + /// + internal bool HasAllInformation { get; } + private ProjectInfo( ProjectId id, VersionStamp version, @@ -112,7 +117,8 @@ public sealed class ProjectInfo IEnumerable analyzerReferences, IEnumerable additionalDocuments, bool isSubmission, - Type hostObjectType) + Type hostObjectType, + bool hasAllInformation) { if (id == null) { @@ -150,28 +156,30 @@ public sealed class ProjectInfo this.AdditionalDocuments = additionalDocuments.ToImmutableReadOnlyListOrEmpty(); this.IsSubmission = isSubmission; this.HostObjectType = hostObjectType; + this.HasAllInformation = hasAllInformation; } /// /// Create a new instance of a ProjectInfo. /// - public static ProjectInfo Create( + internal static ProjectInfo Create( ProjectId id, VersionStamp version, string name, string assemblyName, string language, - string filePath = null, - string outputFilePath = null, - CompilationOptions compilationOptions = null, - ParseOptions parseOptions = null, - IEnumerable documents = null, - IEnumerable projectReferences = null, - IEnumerable metadataReferences = null, - IEnumerable analyzerReferences = null, - IEnumerable additionalDocuments = null, - bool isSubmission = false, - Type hostObjectType = null) + string filePath, + string outputFilePath, + CompilationOptions compilationOptions, + ParseOptions parseOptions, + IEnumerable documents, + IEnumerable projectReferences, + IEnumerable metadataReferences, + IEnumerable analyzerReferences, + IEnumerable additionalDocuments, + bool isSubmission, + Type hostObjectType, + bool hasAllInformation) { return new ProjectInfo( id, @@ -189,7 +197,36 @@ public sealed class ProjectInfo analyzerReferences, additionalDocuments, isSubmission, - hostObjectType); + hostObjectType, + hasAllInformation); + } + + /// + /// Create a new instance of a ProjectInfo. + /// + public static ProjectInfo Create( + ProjectId id, + VersionStamp version, + string name, + string assemblyName, + string language, + string filePath = null, + string outputFilePath = null, + CompilationOptions compilationOptions = null, + ParseOptions parseOptions = null, + IEnumerable documents = null, + IEnumerable projectReferences = null, + IEnumerable metadataReferences = null, + IEnumerable analyzerReferences = null, + IEnumerable additionalDocuments = null, + bool isSubmission = false, + Type hostObjectType = null) + { + return Create( + id, version, name, assemblyName, language, + filePath, outputFilePath, compilationOptions, parseOptions, + documents, projectReferences, metadataReferences, analyzerReferences, additionalDocuments, + isSubmission, hostObjectType, hasAllInformation: true); } private ProjectInfo With( @@ -208,7 +245,8 @@ public sealed class ProjectInfo IEnumerable analyzerReferences = null, IEnumerable additionalDocuments = null, Optional isSubmission = default(Optional), - Optional hostObjectType = default(Optional)) + Optional hostObjectType = default(Optional), + Optional hasAllInformation = default(Optional)) { var newId = id ?? this.Id; var newVersion = version.HasValue ? version.Value : this.Version; @@ -226,6 +264,7 @@ public sealed class ProjectInfo var newAdditionalDocuments = additionalDocuments ?? this.AdditionalDocuments; var newIsSubmission = isSubmission.HasValue ? isSubmission.Value : this.IsSubmission; var newHostObjectType = hostObjectType.HasValue ? hostObjectType.Value : this.HostObjectType; + var newHasAllInformation = hasAllInformation.HasValue ? hasAllInformation.Value : this.HasAllInformation; if (newId == this.Id && newVersion == this.Version && @@ -242,7 +281,8 @@ public sealed class ProjectInfo newAnalyzerReferences == this.AnalyzerReferences && newAdditionalDocuments == this.AdditionalDocuments && newIsSubmission == this.IsSubmission && - newHostObjectType == this.HostObjectType) + newHostObjectType == this.HostObjectType && + newHasAllInformation == this.HasAllInformation) { return this; } @@ -263,7 +303,8 @@ public sealed class ProjectInfo newAnalyzerReferences, newAdditionalDocuments, newIsSubmission, - newHostObjectType); + newHostObjectType, + newHasAllInformation); } public ProjectInfo WithDocuments(IEnumerable documents) @@ -326,6 +367,11 @@ public ProjectInfo WithAnalyzerReferences(IEnumerable analyze return this.With(analyzerReferences: analyzerReferences.ToImmutableReadOnlyListOrEmpty()); } + internal ProjectInfo WithHasAllInformation(bool hasAllInformation) + { + return this.With(hasAllInformation: hasAllInformation); + } + internal string GetDebuggerDisplay() { return nameof(ProjectInfo) + " " + Name + (!string.IsNullOrWhiteSpace(FilePath) ? " " + FilePath : ""); diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs b/src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs index 23736cb631417d550d630c348d73065598d08ef6..2f097a7f1e699670ec58fe04bf7b00babd90cb4b 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs @@ -25,7 +25,9 @@ internal partial class ProjectState private readonly IReadOnlyList _additionalDocumentIds; private readonly AsyncLazy _lazyLatestDocumentVersion; private readonly AsyncLazy _lazyLatestDocumentTopLevelChangeVersion; - private AnalyzerOptions _analyzerOptions; + + // this will be initialized lazily. + private AnalyzerOptions _analyzerOptionsDoNotAccessDirectly; private ProjectState( ProjectInfo projectInfo, @@ -61,12 +63,12 @@ internal ProjectState(ProjectInfo projectInfo, HostLanguageServices languageServ _projectInfo = FixProjectInfo(projectInfo); _documentIds = _projectInfo.Documents.Select(d => d.Id).ToImmutableArray(); - _additionalDocumentIds = this.ProjectInfo.AdditionalDocuments.Select(d => d.Id).ToImmutableArray(); + _additionalDocumentIds = _projectInfo.AdditionalDocuments.Select(d => d.Id).ToImmutableArray(); var docStates = ImmutableDictionary.CreateRange( _projectInfo.Documents.Select(d => new KeyValuePair(d.Id, - CreateDocument(this.ProjectInfo, d, languageServices, solutionServices)))); + CreateDocument(_projectInfo, d, languageServices, solutionServices)))); _documentStates = docStates; @@ -231,12 +233,12 @@ public AnalyzerOptions AnalyzerOptions { get { - if (_analyzerOptions == null) + if (_analyzerOptionsDoNotAccessDirectly == null) { - _analyzerOptions = new AnalyzerOptions(_additionalDocumentStates.Values.Select(d => new AdditionalTextDocument(d)).ToImmutableArray()); + _analyzerOptionsDoNotAccessDirectly = new AnalyzerOptions(_additionalDocumentStates.Values.Select(d => new AdditionalTextDocument(d)).ToImmutableArray()); } - return _analyzerOptions; + return _analyzerOptionsDoNotAccessDirectly; } } @@ -303,6 +305,12 @@ public IReadOnlyList ProjectReferences get { return this.ProjectInfo.ProjectReferences; } } + [DebuggerBrowsable(DebuggerBrowsableState.Collapsed)] + public bool HasAllInformation + { + get { return this.ProjectInfo.HasAllInformation; } + } + [DebuggerBrowsable(DebuggerBrowsableState.Collapsed)] public bool HasDocuments { @@ -457,6 +465,16 @@ public ProjectState UpdateParseOptions(ParseOptions options) documentStates: docMap); } + public ProjectState UpdateHasAllInformation(bool hasAllInformation) + { + if (hasAllInformation == this.HasAllInformation) + { + return this; + } + + return this.With(projectInfo: this.ProjectInfo.WithHasAllInformation(hasAllInformation).WithVersion(this.Version.GetNewerVersion())); + } + public static bool IsSameLanguage(ProjectState project1, ProjectState project2) { return project1.LanguageServices == project2.LanguageServices; diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/Solution.CompilationTracker.State.cs b/src/Workspaces/Core/Portable/Workspace/Solution/Solution.CompilationTracker.State.cs index a7e387d7a5b7a50e9a433213694bde3a9f9ac7fd..7a511b62b5817476d067a7b8d18c021e803df125 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/Solution.CompilationTracker.State.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/Solution.CompilationTracker.State.cs @@ -39,10 +39,10 @@ private class State public ValueSource Compilation { get; } /// - /// Specifies if there are references that got dropped in the production of . This can return + /// Specifies whether and all compilations it depends on contain full information or not. This can return /// null if the state isn't at the point where it would know, and it's necessary to transition to to figure that out. /// - public virtual bool? HasCompleteReferences => null; + public virtual bool? HasSuccessfullyLoadedTransitively => null; /// /// The final compilation if available, otherwise an empty . @@ -135,19 +135,15 @@ public FullDeclarationState(Compilation declarationCompilation) /// private sealed class FinalState : State { - private readonly bool _hasCompleteReferences; + private readonly bool _hasSuccessfullyLoadedTransitively; - public override ValueSource FinalCompilation - { - get { return this.Compilation; } - } - - public override bool? HasCompleteReferences => _hasCompleteReferences; + public override bool? HasSuccessfullyLoadedTransitively => _hasSuccessfullyLoadedTransitively; + public override ValueSource FinalCompilation => this.Compilation; - public FinalState(ValueSource finalCompilationSource, bool hasCompleteReferences) + public FinalState(ValueSource finalCompilationSource, bool hasSuccessfullyLoadedTransitively) : base(finalCompilationSource, finalCompilationSource.GetValue().Clone().RemoveAllReferences()) { - _hasCompleteReferences = hasCompleteReferences; + _hasSuccessfullyLoadedTransitively = hasSuccessfullyLoadedTransitively; } } } diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/Solution.CompilationTracker.cs b/src/Workspaces/Core/Portable/Workspace/Solution/Solution.CompilationTracker.cs index 21b8b2c95b2daeae0f526fcd692713f3b1bba5be..34991d229fe28b80c8ccfac41fe0d02c3abd5e8c 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/Solution.CompilationTracker.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/Solution.CompilationTracker.cs @@ -181,7 +181,7 @@ public CompilationTracker FreezePartialStateWithTree(Solution solution, Document // have the compilation immediately disappear. So we force it to stay around with a ConstantValueSource. // As a policy, all partial-state projects are said to have incomplete references, since the state has no guarantees. return new CompilationTracker(inProgressProject, - new FinalState(new ConstantValueSource(inProgressCompilation), hasCompleteReferences: false)); + new FinalState(new ConstantValueSource(inProgressCompilation), hasSuccessfullyLoadedTransitively: false)); } /// @@ -313,21 +313,6 @@ public Task GetCompilationAsync(Solution solution, CancellationToke } } - public Task HasCompleteReferencesAsync(Solution solution, CancellationToken cancellationToken) - { - var state = this.ReadState(); - - if (state.HasCompleteReferences.HasValue) - { - return Task.FromResult(state.HasCompleteReferences.Value); - } - else - { - return GetOrBuildCompilationInfoAsync(solution, lockGate: true, cancellationToken: cancellationToken) - .ContinueWith(t => t.Result.HasCompleteReferences, cancellationToken, TaskContinuationOptions.ExecuteSynchronously | TaskContinuationOptions.OnlyOnRanToCompletion, TaskScheduler.Default); - } - } - private static string LogBuildCompilationAsync(ProjectState state) { return string.Join(",", state.AssemblyName, state.DocumentIds.Count); @@ -406,7 +391,7 @@ private async Task GetOrBuildDeclarationCompilationAsync(Solution s var finalCompilation = state.FinalCompilation.GetValue(cancellationToken); if (finalCompilation != null) { - return new CompilationInfo(finalCompilation, hasCompleteReferences: state.HasCompleteReferences.Value); + return new CompilationInfo(finalCompilation, state.HasSuccessfullyLoadedTransitively.Value); } // Otherwise, we actually have to build it. Ensure that only one thread is trying to @@ -447,7 +432,7 @@ private async Task GetOrBuildDeclarationCompilationAsync(Solution s var compilation = state.FinalCompilation.GetValue(cancellationToken); if (compilation != null) { - return Task.FromResult(new CompilationInfo(compilation, state.HasCompleteReferences.Value)); + return Task.FromResult(new CompilationInfo(compilation, state.HasSuccessfullyLoadedTransitively.Value)); } compilation = state.Compilation.GetValue(cancellationToken); @@ -582,12 +567,12 @@ private Compilation CreateEmptyCompilation() private struct CompilationInfo { public Compilation Compilation { get; } - public bool HasCompleteReferences { get; } + public bool HasSuccessfullyLoadedTransitively { get; } - public CompilationInfo(Compilation compilation, bool hasCompleteReferences) + public CompilationInfo(Compilation compilation, bool hasSuccessfullyLoadedTransitively) { this.Compilation = compilation; - this.HasCompleteReferences = hasCompleteReferences; + this.HasSuccessfullyLoadedTransitively = hasSuccessfullyLoadedTransitively; } } @@ -602,7 +587,9 @@ public CompilationInfo(Compilation compilation, bool hasCompleteReferences) { try { - bool hasCompleteReferences = true; + // if HasAllInformation is false, then this project is always not completed. + bool hasSuccessfullyLoaded = this.ProjectState.HasAllInformation; + var newReferences = new List(); newReferences.AddRange(this.ProjectState.MetadataReferences); @@ -639,7 +626,7 @@ public CompilationInfo(Compilation compilation, bool hasCompleteReferences) } else { - hasCompleteReferences = false; + hasSuccessfullyLoaded = false; } } } @@ -650,9 +637,10 @@ public CompilationInfo(Compilation compilation, bool hasCompleteReferences) compilation = compilation.WithReferences(newReferences); } - this.WriteState(new FinalState(State.CreateValueSource(compilation, solution.Services), hasCompleteReferences), solution); + bool hasSuccessfullyLoadedTransitively = !HasMissingReferences(compilation, this.ProjectState.MetadataReferences) && await ComputeHasSuccessfullyLoadedTransitivelyAsync(solution, hasSuccessfullyLoaded, cancellationToken).ConfigureAwait(false); - return new CompilationInfo(compilation, hasCompleteReferences); + this.WriteState(new FinalState(State.CreateValueSource(compilation, solution.Services), hasSuccessfullyLoadedTransitively), solution); + return new CompilationInfo(compilation, hasSuccessfullyLoadedTransitively); } catch (Exception e) when (FatalError.ReportUnlessCanceled(e)) { @@ -660,6 +648,43 @@ public CompilationInfo(Compilation compilation, bool hasCompleteReferences) } } + private bool HasMissingReferences(Compilation compilation, IReadOnlyList metadataReferences) + { + foreach (var reference in metadataReferences) + { + if (compilation.GetAssemblyOrModuleSymbol(reference) == null) + { + return true; + } + } + + return false; + } + + private async Task ComputeHasSuccessfullyLoadedTransitivelyAsync(Solution solution, bool hasSuccessfullyLoaded, CancellationToken cancellationToken) + { + if (!hasSuccessfullyLoaded) + { + return false; + } + + foreach (var projectReference in this.ProjectState.ProjectReferences) + { + var project = solution.GetProject(projectReference.ProjectId); + if (project == null) + { + return false; + } + + if (!await solution.HasSuccessfullyLoadedAsync(project, cancellationToken).ConfigureAwait(false)) + { + return false; + } + } + + return true; + } + /// /// Get a metadata reference to this compilation info's compilation with respect to /// another project. For cross language references produce a skeletal assembly. If the @@ -794,6 +819,21 @@ public IEnumerable GetSyntaxTreesWithNameFromDeclarationOnlyCompilat return clone.GetSymbolsWithName(predicate, filter, cancellationToken).SelectMany(s => s.DeclaringSyntaxReferences.Select(r => r.SyntaxTree)); } + public Task HasSuccessfullyLoadedAsync(Solution solution, CancellationToken cancellationToken) + { + var state = this.ReadState(); + + if (state.HasSuccessfullyLoadedTransitively.HasValue) + { + return state.HasSuccessfullyLoadedTransitively.Value ? SpecializedTasks.True : SpecializedTasks.False; + } + else + { + return GetOrBuildCompilationInfoAsync(solution, lockGate: true, cancellationToken: cancellationToken) + .ContinueWith(t => t.Result.HasSuccessfullyLoadedTransitively, cancellationToken, TaskContinuationOptions.ExecuteSynchronously | TaskContinuationOptions.OnlyOnRanToCompletion, TaskScheduler.Default); + } + } + #region Versions // Dependent Versions are stored on compilation tracker so they are more likely to survive when unrelated solution branching occurs. diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs b/src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs index 308366c16cdbc493503fbfbbdc7a7e100b81f691..81f43389d5c8429a35dc267b06ddb061060cadfa 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs @@ -3,8 +3,6 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; -using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Runtime.CompilerServices; using System.Threading; @@ -839,6 +837,32 @@ public Solution WithProjectParseOptions(ProjectId projectId, ParseOptions option } } + /// + /// Create a new solution instance with the project specified updated to have + /// the specified hasAllInformation. + /// + // TODO: make it public + internal Solution WithHasAllInformation(ProjectId projectId, bool hasAllInformation) + { + if (projectId == null) + { + throw new ArgumentNullException(nameof(projectId)); + } + + Contract.Requires(this.ContainsProject(projectId)); + + var oldProject = this.GetProjectState(projectId); + var newProject = oldProject.UpdateHasAllInformation(hasAllInformation); + + if (oldProject == newProject) + { + return this; + } + + // fork without any change on compilation. + return this.ForkProject(newProject); + } + private static async Task ReplaceSyntaxTreesWithTreesFromNewProjectStateAsync(Compilation compilation, ProjectState projectState, CancellationToken cancellationToken) { var syntaxTrees = new List(capacity: projectState.DocumentIds.Count); @@ -2020,11 +2044,16 @@ internal Task GetCompilationAsync(Project project, CancellationToke : SpecializedTasks.Default(); } - internal Task HasCompleteReferencesAsync(Project project, CancellationToken cancellationToken) + /// + /// Return reference completeness for the given project and all projects this references. + /// + internal Task HasSuccessfullyLoadedAsync(Project project, CancellationToken cancellationToken) { + // return HasAllInformation when compilation is not supported. + // regardless whether project support compilation or not, if projectInfo is not complete, we can't gurantee its reference completeness return project.SupportsCompilation - ? this.GetCompilationTracker(project.Id).HasCompleteReferencesAsync(this, cancellationToken) - : SpecializedTasks.False; + ? this.GetCompilationTracker(project.Id).HasSuccessfullyLoadedAsync(this, cancellationToken) + : project.Solution.GetProjectState(project.Id).HasAllInformation ? SpecializedTasks.True : SpecializedTasks.False; } private static readonly ConditionalWeakTable s_metadataReferenceToProjectMap = diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace.cs b/src/Workspaces/Core/Portable/Workspace/Workspace.cs index 1e16e31ecf396f5d6d42b908b06c792763f66981..ca6875e7f266884a96f04e68ecbb0d4a908e096f 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace.cs @@ -703,6 +703,25 @@ protected internal void OnAdditionalDocumentTextLoaderChanged(DocumentId documen } } + /// + /// Call this method when status of project has changed to incomplete. + /// See for more information. + /// + // TODO: make it public + internal void OnHasAllInformationChanged(ProjectId projectId, bool hasAllInformation) + { + using (_serializationLock.DisposableWait()) + { + CheckProjectIsInCurrentSolution(projectId); + + // if state is different than what we have + var oldSolution = this.CurrentSolution; + var newSolution = this.SetCurrentSolution(oldSolution.WithHasAllInformation(projectId, hasAllInformation)); + + this.RaiseWorkspaceChangedEventAsync(WorkspaceChangeKind.ProjectChanged, oldSolution, newSolution, projectId); + } + } + /// /// Call this method when the text of a document is updated in the host environment. /// diff --git a/src/Workspaces/Core/Portable/Workspaces.csproj b/src/Workspaces/Core/Portable/Workspaces.csproj index 3320ce17619202e41baa988ec77618571beffc66..caa900d6baf5dc590cca9a21be53b434d9e8c5bd 100644 --- a/src/Workspaces/Core/Portable/Workspaces.csproj +++ b/src/Workspaces/Core/Portable/Workspaces.csproj @@ -386,6 +386,7 @@ + diff --git a/src/Workspaces/CoreTest/SolutionTests/SolutionTests.cs b/src/Workspaces/CoreTest/SolutionTests/SolutionTests.cs index fd5e8eb8ac7c89626130269306b0d29ef00d43e7..508d7341c68db7c8175586febac6e8a6565f479f 100644 --- a/src/Workspaces/CoreTest/SolutionTests/SolutionTests.cs +++ b/src/Workspaces/CoreTest/SolutionTests/SolutionTests.cs @@ -1404,8 +1404,8 @@ public void TestProjectWithNoBrokenReferencesHasNoIncompleteReferences() projectReferences: new[] { new ProjectReference(project1.Id) })); // Nothing should have incomplete references, and everything should build - Assert.True(project1.HasCompleteReferencesAsync().Result); - Assert.True(project2.HasCompleteReferencesAsync().Result); + Assert.True(project1.HasSuccessfullyLoadedAsync().Result); + Assert.True(project2.HasSuccessfullyLoadedAsync().Result); Assert.Single(project2.GetCompilationAsync().Result.ExternalReferences); } @@ -1425,8 +1425,8 @@ public void TestProjectWithBrokenCrossLanguageReferenceHasIncompleteReferences() LanguageNames.VisualBasic, projectReferences: new[] { new ProjectReference(project1.Id) })); - Assert.True(project1.HasCompleteReferencesAsync().Result); - Assert.False(project2.HasCompleteReferencesAsync().Result); + Assert.True(project1.HasSuccessfullyLoadedAsync().Result); + Assert.False(project2.HasSuccessfullyLoadedAsync().Result); Assert.Empty(project2.GetCompilationAsync().Result.ExternalReferences); } @@ -1450,8 +1450,103 @@ public void TestFrozenPartialProjectAlwaysIsIncomplete() // Nothing should have incomplete references, and everything should build var frozenSolution = document.WithFrozenPartialSemanticsAsync(CancellationToken.None).Result.Project.Solution; - Assert.True(frozenSolution.GetProject(project1.Id).HasCompleteReferencesAsync().Result); - Assert.True(frozenSolution.GetProject(project2.Id).HasCompleteReferencesAsync().Result); + Assert.True(frozenSolution.GetProject(project1.Id).HasSuccessfullyLoadedAsync().Result); + Assert.True(frozenSolution.GetProject(project2.Id).HasSuccessfullyLoadedAsync().Result); + } + + [Fact] + public void TestProjectCompletenessWithMultipleProjects() + { + Project csBrokenProject; + Project vbNormalProject; + Project dependsOnBrokenProject; + Project dependsOnVbNormalProject; + Project transitivelyDependsOnBrokenProjects; + Project transitivelyDependsOnNormalProjects; + GetMultipleProjects(out csBrokenProject, out vbNormalProject, out dependsOnBrokenProject, out dependsOnVbNormalProject, out transitivelyDependsOnBrokenProjects, out transitivelyDependsOnNormalProjects); + + // check flag for a broken project itself + Assert.False(csBrokenProject.HasSuccessfullyLoadedAsync().Result); + + // check flag for a normal project itself + Assert.True(vbNormalProject.HasSuccessfullyLoadedAsync().Result); + + // check flag for normal project that directly reference a broken project + Assert.False(dependsOnBrokenProject.HasSuccessfullyLoadedAsync().Result); + + // check flag for normal project that directly reference only normal project + Assert.True(dependsOnVbNormalProject.HasSuccessfullyLoadedAsync().Result); + + // check flag for normal project that indirectly reference a borken project + // normal project -> normal project -> broken project + Assert.False(transitivelyDependsOnBrokenProjects.HasSuccessfullyLoadedAsync().Result); + + // check flag for normal project that indirectly reference only normal project + // normal project -> normal project -> normal project + Assert.True(transitivelyDependsOnNormalProjects.HasSuccessfullyLoadedAsync().Result); + } + + private static void GetMultipleProjects( + out Project csBrokenProject, + out Project vbNormalProject, + out Project dependsOnBrokenProject, + out Project dependsOnVbNormalProject, + out Project transitivelyDependsOnBrokenProjects, + out Project transitivelyDependsOnNormalProjects) + { + var workspace = new AdhocWorkspace(); + + csBrokenProject = workspace.AddProject( + ProjectInfo.Create( + ProjectId.CreateNewId(), + VersionStamp.Create(), + "CSharpProject", + "CSharpProject", + LanguageNames.CSharp).WithHasAllInformation(hasAllInformation: false)); + + vbNormalProject = workspace.AddProject( + ProjectInfo.Create( + ProjectId.CreateNewId(), + VersionStamp.Create(), + "VisualBasicProject", + "VisualBasicProject", + LanguageNames.VisualBasic)); + + dependsOnBrokenProject = workspace.AddProject( + ProjectInfo.Create( + ProjectId.CreateNewId(), + VersionStamp.Create(), + "VisualBasicProject", + "VisualBasicProject", + LanguageNames.VisualBasic, + projectReferences: new[] { new ProjectReference(csBrokenProject.Id), new ProjectReference(vbNormalProject.Id) })); + + dependsOnVbNormalProject = workspace.AddProject( + ProjectInfo.Create( + ProjectId.CreateNewId(), + VersionStamp.Create(), + "CSharpProject", + "CSharpProject", + LanguageNames.CSharp, + projectReferences: new[] { new ProjectReference(vbNormalProject.Id) })); + + transitivelyDependsOnBrokenProjects = workspace.AddProject( + ProjectInfo.Create( + ProjectId.CreateNewId(), + VersionStamp.Create(), + "CSharpProject", + "CSharpProject", + LanguageNames.CSharp, + projectReferences: new[] { new ProjectReference(dependsOnBrokenProject.Id) })); + + transitivelyDependsOnNormalProjects = workspace.AddProject( + ProjectInfo.Create( + ProjectId.CreateNewId(), + VersionStamp.Create(), + "VisualBasicProject", + "VisualBasicProject", + LanguageNames.VisualBasic, + projectReferences: new[] { new ProjectReference(dependsOnVbNormalProject.Id) })); } } }