Add support for compilation end code fix.

now code fix should support diagnostics from compilation end action.

a few options are added to test experience which can be changed from Tools/Options/Roslyn/Diagnostic pane.

src/EditorFeatures/Core/Implementation/TodoComment/TodoCommentState.cs

@@ -87,7 +87,7 @@ protected override void WriteTo(Stream stream, Data data, CancellationToken canc
             public ImmutableArray<ITaskItem> GetItems_TestingOnly(DocumentId documentId)
             {
                 Data data;
-                if (this.DataCache.TryGetValue(documentId, out data))
+                if (this.DataCache.TryGetValue(documentId, out data) && data != null)
                 {
                     return data.Items;
                 }

src/EditorFeatures/Test2/Diagnostics/DiagnosticServiceTests.vb

@@ -8,6 +8,7 @@ Imports Microsoft.CodeAnalysis.Diagnostics
 Imports Microsoft.CodeAnalysis.Diagnostics.EngineV1
 Imports Microsoft.CodeAnalysis.Editor.UnitTests.Diagnostics
 Imports Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces
+Imports Microsoft.CodeAnalysis.Options
 Imports Microsoft.CodeAnalysis.Test.Utilities
 Imports Microsoft.CodeAnalysis.Text
 Imports Roslyn.Utilities
@@ -594,6 +595,11 @@ class AnonymousFunctions
 
             Using workspace = TestWorkspaceFactory.CreateWorkspace(test)
                 Dim project = workspace.CurrentSolution.Projects.Single()
+
+                ' turn off heuristic
+                Dim options = workspace.Services.GetService(Of IOptionService)()
+                options.SetOptions(options.GetOptions.WithChangedOption(InternalDiagnosticsOptions.UseCompilationEndCodeFixHueristic, False))
+
                 Dim analyzer = New CompilationEndedAnalyzer
                 Dim analyzerReference = New AnalyzerImageReference(ImmutableArray.Create(Of DiagnosticAnalyzer)(analyzer))
                 project = project.AddAnalyzerReference(analyzerReference)
@@ -604,17 +610,18 @@ class AnonymousFunctions
                 Dim descriptorsMap = diagnosticService.GetDiagnosticDescriptors(project)
                 Assert.Equal(1, descriptorsMap.Count)
 
-                ' Ask for document diagnostics multiple times, and verify compilation diagnostics are not reported.
+                ' Ask for document diagnostics multiple times, and verify compilation diagnostics are reported.
                 Dim document = project.Documents.Single()
+
                 Dim fullSpan = document.GetSyntaxRootAsync().WaitAndGetResult(CancellationToken.None).FullSpan
                 Dim diagnostics = diagnosticService.GetDiagnosticsForSpanAsync(document, fullSpan, CancellationToken.None).WaitAndGetResult(CancellationToken.None)
-                Assert.Equal(0, diagnostics.Count())
+                Assert.Equal(1, diagnostics.Count())
 
                 diagnostics = diagnosticService.GetDiagnosticsForSpanAsync(document, fullSpan, CancellationToken.None).WaitAndGetResult(CancellationToken.None)
-                Assert.Equal(0, diagnostics.Count())
+                Assert.Equal(1, diagnostics.Count())
 
                 diagnostics = diagnosticService.GetDiagnosticsForSpanAsync(document, fullSpan, CancellationToken.None).WaitAndGetResult(CancellationToken.None)
-                Assert.Equal(0, diagnostics.Count())
+                Assert.Equal(1, diagnostics.Count())
 
                 ' Verify compilation diagnostics are reported with correct location info when asked for project diagnostics.
                 Dim projectDiagnostics = diagnosticService.GetProjectDiagnosticsForIdsAsync(project.Solution, project.Id).WaitAndGetResult(CancellationToken.None)

src/Features/Core/Diagnostics/EngineV1/DiagnosticAnalyzerDriver.cs

@@ -29,8 +29,6 @@ internal class DiagnosticAnalyzerDriver
         private readonly AbstractHostDiagnosticUpdateSource _hostDiagnosticUpdateSource;
         private readonly CancellationToken _cancellationToken;
         private readonly ISyntaxNodeAnalyzerService _syntaxNodeAnalyzerService;
-        private readonly Dictionary<SyntaxNode, ImmutableArray<SyntaxNode>> _descendantExecutableNodesMap;
-        private readonly ISyntaxFactsService _syntaxFacts;
         private readonly IGeneratedCodeRecognitionService _generatedCodeService;
         private readonly IAnalyzerDriverService _analyzerDriverService;
 
@@ -74,8 +72,6 @@ public DiagnosticAnalyzerDriver(Project project, LogAggregator logAggregator, Ab
             _syntaxNodeAnalyzerService = syntaxNodeAnalyzerService;
             _hostDiagnosticUpdateSource = hostDiagnosticUpdateSource;
             _cancellationToken = cancellationToken;
-            _descendantExecutableNodesMap = new Dictionary<SyntaxNode, ImmutableArray<SyntaxNode>>();
-            _syntaxFacts = document.Project.LanguageServices.GetService<ISyntaxFactsService>();
             _generatedCodeService = document.Project.Solution.Workspace.Services.GetService<IGeneratedCodeRecognitionService>();
             _analyzerDriverService = document.Project.LanguageServices.GetService<IAnalyzerDriverService>();
             _analyzerOptions = new WorkspaceAnalyzerOptions(_project.AnalyzerOptions, _project.Solution.Workspace);
@@ -97,7 +93,6 @@ public DiagnosticAnalyzerDriver(Project project, LogAggregator logAggregator, Ab
             _generatedCodeService = project.Solution.Workspace.Services.GetService<IGeneratedCodeRecognitionService>();
             _analyzerDriverService = project.LanguageServices.GetService<IAnalyzerDriverService>();
             _hostDiagnosticUpdateSource = hostDiagnosticUpdateSource;
-            _descendantExecutableNodesMap = null;
             _analyzerOptions = new WorkspaceAnalyzerOptions(_project.AnalyzerOptions, _project.Solution.Workspace);
             _onAnalyzerException = overriddenOnAnalyzerException ?? Default_OnAnalyzerException;
             _onAnalyzerException_NoTelemetryLogging = overriddenOnAnalyzerException ?? Default_OnAnalyzerException_NoTelemetryLogging;

src/Features/Core/Diagnostics/EngineV1/DiagnosticIncrementalAnalyzer.DiagnosticState.cs

@@ -128,15 +128,14 @@ private AnalysisData TryGetExistingData(Stream stream, Project project, Document
                         var textVersion = VersionStamp.ReadFrom(reader);
                         var dataVersion = VersionStamp.ReadFrom(reader);
 
-                        // textversion can be default for document from project analysis.
-                        if (dataVersion == VersionStamp.Default)
+                        if (textVersion == VersionStamp.Default || dataVersion == VersionStamp.Default)
                         {
                             return null;
                         }
 
                         AppendItems(reader, project, document, list, cancellationToken);
 
-                        return new AnalysisData(textVersion, dataVersion, list.ToImmutableArray<DiagnosticData>());
+                        return new AnalysisData(textVersion, dataVersion, list.ToImmutableArray());
                     }
                 }
                 catch (Exception)

src/Features/Core/Diagnostics/EngineV1/DiagnosticIncrementalAnalyzer.IncrementalAnalyzer.AnalyzerExecutor.cs

@@ -121,16 +121,13 @@ public async Task<AnalysisData> GetProjectAnalysisDataAsync(DiagnosticAnalyzerDr
                     var state = stateSet.GetState(StateType.Project);
                     var existingData = await GetExistingProjectAnalysisDataAsync(project, state, cancellationToken).ConfigureAwait(false);
 
-                    // TODO: 
-                    // if there is any document level result, we can't ever use cache since we can't track those changes in current design
-                    // hopely v2 engine, either don't care this at all, or can deal with this better
-                    if (CheckSemanticVersions(project, existingData, versions) && !existingData.Items.Any(d => d.DocumentId != null))
+                    if (CheckSemanticVersions(project, existingData, versions))
                     {
                         return existingData;
                     }
 
                     var diagnosticData = await GetProjectDiagnosticsAsync(analyzerDriver, stateSet.Analyzer, _owner.ForceAnalyzeAllDocuments).ConfigureAwait(false);
-                    return new AnalysisData(VersionStamp.Default, versions.DataVersion, GetExistingItems(existingData), diagnosticData.AsImmutableOrEmpty());
+                    return new AnalysisData(versions.TextVersion, versions.DataVersion, GetExistingItems(existingData), diagnosticData.AsImmutableOrEmpty());
                 }
                 catch (Exception e) when (FatalError.ReportUnlessCanceled(e))
                 {
@@ -140,19 +137,22 @@ public async Task<AnalysisData> GetProjectAnalysisDataAsync(DiagnosticAnalyzerDr
 
             private async Task<AnalysisData> GetExistingProjectAnalysisDataAsync(Project project, DiagnosticState state, CancellationToken cancellationToken)
             {
-                var dataVersion = VersionStamp.Default;
-                var existingData = await state.TryGetExistingDataAsync(project, cancellationToken).ConfigureAwait(false);
-
-                // quick path.
-                if (existingData == null || existingData.Items.Length == 0)
+                // quick bail out
+                if (state.Count == 0)
                 {
-                    return existingData;
+                    return null;
                 }
 
+                var textVersion = VersionStamp.Default;
+                var dataVersion = VersionStamp.Default;
+                var existingData = await state.TryGetExistingDataAsync(project, cancellationToken).ConfigureAwait(false);
+
                 var builder = ImmutableArray.CreateBuilder<DiagnosticData>();
                 if (existingData != null)
                 {
+                    textVersion = existingData.TextVersion;
                     dataVersion = existingData.DataVersion;
+
                     builder.AddRange(existingData.Items);
                 }
 
@@ -169,16 +169,19 @@ private async Task<AnalysisData> GetExistingProjectAnalysisDataAsync(Project pro
                         continue;
                     }
 
+                    textVersion = existingData.TextVersion;
                     dataVersion = existingData.DataVersion;
+
                     builder.AddRange(existingData.Items);
                 }
 
                 if (dataVersion == VersionStamp.Default)
                 {
+                    Contract.Requires(textVersion == VersionStamp.Default);
                     return null;
                 }
 
-                return new AnalysisData(VersionStamp.Default, dataVersion, builder.ToImmutable());
+                return new AnalysisData(textVersion, dataVersion, builder.ToImmutable());
             }
 
             private bool CanUseDocumentState(AnalysisData existingData, VersionStamp textVersion, VersionStamp dataVersion)

src/Features/Core/Diagnostics/EngineV1/DiagnosticIncrementalAnalyzer.StateSet.cs

@@ -36,11 +36,11 @@ public DiagnosticState GetState(StateType stateType)
                 return _state[(int)stateType];
             }
 
-            public void Remove(object key)
+            public void Remove(object documentOrProjectId)
             {
                 for (var stateType = 0; stateType < s_stateTypeCount; stateType++)
                 {
-                    _state[stateType].Remove(key);
+                    _state[stateType].Remove(documentOrProjectId);
                 }
             }
 

src/Features/Core/Diagnostics/EngineV1/DiagnosticIncrementalAnalyzer_GetDiagnostics.cs

@@ -396,9 +396,10 @@ protected async Task<VersionArgument> GetVersionsAsync(object documentOrProject,
                     case StateType.Project:
                         {
                             var project = (Project)documentOrProject;
+                            var projectTextVersion = await project.GetLatestDocumentVersionAsync(cancellationToken).ConfigureAwait(false);
                             var semanticVersion = await project.GetDependentSemanticVersionAsync(cancellationToken).ConfigureAwait(false);
                             var projectVersion = await project.GetDependentVersionAsync(cancellationToken).ConfigureAwait(false);
-                            return new VersionArgument(VersionStamp.Default, semanticVersion, projectVersion);
+                            return new VersionArgument(projectTextVersion, semanticVersion, projectVersion);
                         }
 
                     default:

src/Features/Core/Diagnostics/EngineV1/DiagnosticIncrementalAnalyzer_GetLatestDiagnosticsForSpan.cs

+// 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.Threading;
+using System.Threading.Tasks;
+using Microsoft.CodeAnalysis.ErrorReporting;
+using Microsoft.CodeAnalysis.Text;
+using Roslyn.Utilities;
+
+namespace Microsoft.CodeAnalysis.Diagnostics.EngineV1
+{
+    internal partial class DiagnosticIncrementalAnalyzer : BaseDiagnosticIncrementalAnalyzer
+    {
+        private class LatestDiagnosticsForSpanGetter
+        {
+            private readonly DiagnosticIncrementalAnalyzer _owner;
+
+            private readonly Document _document;
+            private readonly TextSpan _range;
+            private readonly bool _blockForData;
+            private readonly CancellationToken _cancellationToken;
+
+            private readonly DiagnosticAnalyzerDriver _spanBasedDriver;
+            private readonly DiagnosticAnalyzerDriver _documentBasedDriver;
+            private readonly DiagnosticAnalyzerDriver _projectDriver;
+
+            private readonly List<DiagnosticData> _diagnostics;
+
+            public LatestDiagnosticsForSpanGetter(
+                DiagnosticIncrementalAnalyzer owner, Document document, SyntaxNode root, TextSpan range, bool blockForData, CancellationToken cancellationToken) :
+                this(owner, document, root, range, blockForData, new List<DiagnosticData>(), cancellationToken)
+            {
+            }
+
+            public LatestDiagnosticsForSpanGetter(
+                DiagnosticIncrementalAnalyzer owner, Document document, SyntaxNode root, TextSpan range, bool blockForData, List<DiagnosticData> diagnostics, CancellationToken cancellationToken)
+            {
+                _owner = owner;
+
+                _document = document;
+                _range = range;
+                _blockForData = blockForData;
+                _cancellationToken = cancellationToken;
+
+                _diagnostics = diagnostics;
+
+                // Share the diagnostic analyzer driver across all analyzers.
+                var fullSpan = root == null ? null : (TextSpan?)root.FullSpan;
+
+                _spanBasedDriver = new DiagnosticAnalyzerDriver(_document, _range, root, _owner._diagnosticLogAggregator, _owner.HostDiagnosticUpdateSource, _cancellationToken);
+                _documentBasedDriver = new DiagnosticAnalyzerDriver(_document, fullSpan, root, _owner._diagnosticLogAggregator, _owner.HostDiagnosticUpdateSource, _cancellationToken);
+                _projectDriver = new DiagnosticAnalyzerDriver(_document.Project, _owner._diagnosticLogAggregator, _owner.HostDiagnosticUpdateSource, _cancellationToken);
+            }
+
+            public List<DiagnosticData> Diagnostics => _diagnostics;
+
+            public async Task<bool> TryGetAsync()
+            {
+                try
+                {
+                    var textVersion = await _document.GetTextVersionAsync(_cancellationToken).ConfigureAwait(false);
+                    var syntaxVersion = await _document.GetSyntaxVersionAsync(_cancellationToken).ConfigureAwait(false);
+                    var projectTextVersion = await _document.Project.GetLatestDocumentVersionAsync(_cancellationToken).ConfigureAwait(false);
+                    var semanticVersion = await _document.Project.GetDependentSemanticVersionAsync(_cancellationToken).ConfigureAwait(false);
+
+                    var result = true;
+                    foreach (var stateSet in _owner._stateManger.GetOrCreateStateSets(_document.Project))
+                    {
+                        result &= await TryGetDocumentDiagnosticsAsync(
+                            stateSet, StateType.Syntax, (t, d) => t.Equals(textVersion) && d.Equals(syntaxVersion), GetSyntaxDiagnosticsAsync).ConfigureAwait(false);
+
+                        result &= await TryGetDocumentDiagnosticsAsync(
+                            stateSet, StateType.Document, (t, d) => t.Equals(textVersion) && d.Equals(semanticVersion), GetSemanticDiagnosticsAsync).ConfigureAwait(false);
+
+                        // check whether compilation end code fix is enabled
+                        if (!_document.Project.Solution.Workspace.Options.GetOption(InternalDiagnosticsOptions.CompilationEndCodeFix))
+                        {
+                            continue;
+                        }
+
+                        // check whether hueristic is enabled
+                        if (_blockForData && _document.Project.Solution.Workspace.Options.GetOption(InternalDiagnosticsOptions.UseCompilationEndCodeFixHueristic))
+                        {
+                            var analysisData = await stateSet.GetState(StateType.Project).TryGetExistingDataAsync(_document, _cancellationToken).ConfigureAwait(false);
+
+                            // no previous compilation end diagnostics in this file.
+                            if (analysisData == null || analysisData.Items.Length == 0 ||
+                                !analysisData.TextVersion.Equals(projectTextVersion) ||
+                                !analysisData.DataVersion.Equals(semanticVersion))
+                            {
+                                continue;
+                            }
+                        }
+
+                        result &= await TryGetDocumentDiagnosticsAsync(
+                            stateSet, StateType.Project, (t, d) => t.Equals(projectTextVersion) && d.Equals(semanticVersion), GetProjectDiagnosticsWorkerAsync).ConfigureAwait(false);
+                    }
+
+                    return result;
+                }
+                catch (Exception e) when (FatalError.ReportUnlessCanceled(e))
+                {
+                    throw ExceptionUtilities.Unreachable;
+                }
+            }
+
+            private async Task<bool> TryGetDocumentDiagnosticsAsync(
+                StateSet stateSet, StateType stateType, Func<VersionStamp, VersionStamp, bool> versionCheck,
+                Func<DiagnosticAnalyzerDriver, DiagnosticAnalyzer, Task<IEnumerable<DiagnosticData>>> getDiagnostics)
+            {
+                bool supportsSemanticInSpan;
+                if (_spanBasedDriver.IsAnalyzerSuppressed(stateSet.Analyzer) ||
+                    !ShouldRunAnalyzerForStateType(stateSet, stateType, out supportsSemanticInSpan))
+                {
+                    return true;
+                }
+
+                var analyzerDriver = GetAnalyzerDriverBasedOnStateType(stateType, supportsSemanticInSpan);
+
+                var shouldInclude = (Func<DiagnosticData, bool>)(d => d.DocumentId == _document.Id && _range.IntersectsWith(d.TextSpan));
+
+                // make sure we get state even when none of our analyzer has ran yet. 
+                // but this shouldn't create analyzer that doesnt belong to this project (language)
+                var state = stateSet.GetState(stateType);
+
+                // see whether we can use existing info
+                var existingData = await state.TryGetExistingDataAsync(_document, _cancellationToken).ConfigureAwait(false);
+                if (existingData != null && versionCheck(existingData.TextVersion, existingData.DataVersion))
+                {
+                    if (existingData.Items == null || existingData.Items.Length == 0)
+                    {
+                        return true;
+                    }
+
+                    _diagnostics.AddRange(existingData.Items.Where(shouldInclude));
+                    return true;
+                }
+
+                // check whether we want up-to-date document wide diagnostics
+                if (!BlockForData(stateType, supportsSemanticInSpan))
+                {
+                    return false;
+                }
+
+                var dx = await getDiagnostics(analyzerDriver, stateSet.Analyzer).ConfigureAwait(false);
+                if (dx != null)
+                {
+                    // no state yet
+                    _diagnostics.AddRange(dx.Where(shouldInclude));
+                }
+
+                return true;
+            }
+
+            private bool ShouldRunAnalyzerForStateType(StateSet stateSet, StateType stateType, out bool supportsSemanticInSpan)
+            {
+                if (stateType == StateType.Project)
+                {
+                    return DiagnosticIncrementalAnalyzer.ShouldRunAnalyzerForStateType(_projectDriver, stateSet.Analyzer, stateType, out supportsSemanticInSpan);
+                }
+
+                return DiagnosticIncrementalAnalyzer.ShouldRunAnalyzerForStateType(_spanBasedDriver, stateSet.Analyzer, stateType, out supportsSemanticInSpan);
+            }
+
+            private bool BlockForData(StateType stateType, bool supportsSemanticInSpan)
+            {
+                if (stateType == StateType.Document && !supportsSemanticInSpan && !_blockForData)
+                {
+                    return false;
+                }
+
+                if (stateType == StateType.Project && !_blockForData)
+                {
+                    return false;
+                }
+
+                // TODO:
+                // this probably need to change in v2 engine. but in v1 engine, we have assumption that all syntax related action
+                // will return diagnostics that only belong to given span
+                return true;
+            }
+
+            private DiagnosticAnalyzerDriver GetAnalyzerDriverBasedOnStateType(StateType stateType, bool supportsSemanticInSpan)
+            {
+                return stateType == StateType.Project ? _projectDriver : supportsSemanticInSpan ? _spanBasedDriver : _documentBasedDriver;
+            }
+
+            private Task<IEnumerable<DiagnosticData>> GetProjectDiagnosticsWorkerAsync(DiagnosticAnalyzerDriver driver, DiagnosticAnalyzer analyzer)
+            {
+                return GetProjectDiagnosticsAsync(driver, analyzer, _owner.ForceAnalyzeAllDocuments);
+            }
+        }
+    }
+}

src/Features/Core/Diagnostics/InternalDiagnosticsOptions.cs

@@ -13,5 +13,11 @@ internal static class InternalDiagnosticsOptions
 
         [ExportOption]
         public static readonly Option<bool> UseDiagnosticEngineV2 = new Option<bool>(OptionName, "Use Diagnostic Engine V2", defaultValue: false);
+
+        [ExportOption]
+        public static readonly Option<bool> CompilationEndCodeFix = new Option<bool>(OptionName, "Enable Compilation End Code Fix", defaultValue: true);
+
+        [ExportOption]
+        public static readonly Option<bool> UseCompilationEndCodeFixHueristic = new Option<bool>(OptionName, "Enable Compilation End Code Fix Only If There is existing one", defaultValue: true);
     }
 }

src/Features/Core/Features.csproj

@@ -184,6 +184,7 @@
     <Compile Include="Diagnostics\EngineV1\DiagnosticIncrementalAnalyzer.StateManager.HostStates.cs" />
     <Compile Include="Diagnostics\EngineV1\DiagnosticIncrementalAnalyzer.StateManager.ProjectStates.cs" />
     <Compile Include="Diagnostics\EngineV1\DiagnosticIncrementalAnalyzer.StateSet.cs" />
+    <Compile Include="Diagnostics\EngineV1\DiagnosticIncrementalAnalyzer_GetLatestDiagnosticsForSpan.cs" />
     <Compile Include="Diagnostics\HostAnalyzerManager.cs" />
     <Compile Include="Diagnostics\Analyzers\IDEDiagnosticIds.cs" />
     <Compile Include="Diagnostics\BaseDiagnosticIncrementalAnalyzer.cs" />

src/Features/Core/SolutionCrawler/State/AbstractAnalyzerState.cs

@@ -11,7 +11,7 @@ namespace Microsoft.CodeAnalysis.SolutionCrawler.State
 {
     internal abstract class AbstractAnalyzerState<TKey, TValue, TData>
     {
-        protected readonly ConcurrentDictionary<TKey, TData> DataCache = new ConcurrentDictionary<TKey, TData>();
+        protected readonly ConcurrentDictionary<TKey, TData> DataCache = new ConcurrentDictionary<TKey, TData>(concurrencyLevel: 2, capacity: 10);
 
         protected abstract TKey GetCacheKey(TValue value);
         protected abstract Solution GetSolution(TValue value);
@@ -23,15 +23,24 @@ internal abstract class AbstractAnalyzerState<TKey, TValue, TData>
         protected abstract void WriteTo(Stream stream, TData data, CancellationToken cancellationToken);
         protected abstract Task<bool> WriteStreamAsync(IPersistentStorage storage, TValue value, Stream stream, CancellationToken cancellationToken);
 
+        public int Count { get { return this.DataCache.Count; } }
+
         public async Task<TData> TryGetExistingDataAsync(TValue value, CancellationToken cancellationToken)
         {
-            // we have data for the document
             TData data;
-            if (this.DataCache.TryGetValue(GetCacheKey(value), out data))
+            if (!this.DataCache.TryGetValue(GetCacheKey(value), out data))
+            {
+                // we don't have data
+                return default(TData);
+            }
+
+            // we have in memory cache for the document
+            if (!object.Equals(data, default(TData)))
             {
                 return data;
             }
 
+            // we have persisted data
             var solution = GetSolution(value);
             var persistService = solution.Workspace.Services.GetService<IPersistentStorageService>();
 
@@ -55,13 +64,7 @@ public async Task PersistAsync(TValue value, TData data, CancellationToken cance
 
             // if data is for opened document or if persistence failed, 
             // we keep small cache so that we don't pay cost of deserialize/serializing data that keep changing
-            if (!succeeded || ShouldCache(value))
-            {
-                this.DataCache[id] = data;
-                return;
-            }
-
-            Remove(id);
+            this.DataCache[id] = (!succeeded || ShouldCache(value)) ? data : default(TData);
         }
 
         public bool Remove(TKey id)