diff --git a/src/Microsoft.CodeAnalysis.AnalyzerUtilities/PublicAPI.Unshipped.txt b/src/Microsoft.CodeAnalysis.AnalyzerUtilities/PublicAPI.Unshipped.txt index c05b724b0d..4fc097aee7 100644 --- a/src/Microsoft.CodeAnalysis.AnalyzerUtilities/PublicAPI.Unshipped.txt +++ b/src/Microsoft.CodeAnalysis.AnalyzerUtilities/PublicAPI.Unshipped.txt @@ -1,6 +1,7 @@ #nullable enable abstract Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AbstractDataFlowAnalysisContext.ComputeEqualsByHashCodeParts(Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AbstractDataFlowAnalysisContext! obj) -> bool abstract Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AbstractDataFlowAnalysisContext.ComputeHashCodePartsSpecific(ref Analyzer.Utilities.RoslynHashCode hashCode) -> void +abstract Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AnalysisEntityBasedPredicateAnalysisData.ValueDomain.get -> Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.AbstractValueDomain! abstract Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.CacheBasedEquatable.ComputeEqualsByHashCodeParts(Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.CacheBasedEquatable! obj) -> bool abstract Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.CacheBasedEquatable.ComputeHashCodeParts(ref Analyzer.Utilities.RoslynHashCode hashCode) -> void abstract Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.DataFlowOperationVisitor.GetAbstractDefaultValue(Microsoft.CodeAnalysis.ITypeSymbol? type) -> TAbstractAnalysisValue diff --git a/src/NetAnalyzers/UnitTests/Microsoft.CodeQuality.Analyzers/Maintainability/AvoidDeadConditionalCode_NullAnalysis.cs b/src/NetAnalyzers/UnitTests/Microsoft.CodeQuality.Analyzers/Maintainability/AvoidDeadConditionalCode_NullAnalysis.cs index dbd3df3203..df8647a5d6 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.CodeQuality.Analyzers/Maintainability/AvoidDeadConditionalCode_NullAnalysis.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.CodeQuality.Analyzers/Maintainability/AvoidDeadConditionalCode_NullAnalysis.cs @@ -6762,6 +6762,103 @@ public void M(S[] arguments) }"); } + [Trait(Traits.DataflowAnalysis, Traits.Dataflow.NullAnalysis)] + [Trait(Traits.DataflowAnalysis, Traits.Dataflow.PointsToAnalysis)] + [Fact, WorkItem(6616, "https://github.com/dotnet/roslyn-analyzers/issues/6616")] + public async Task IndexedArrayAccessWithConstantsAndNonConstants_NoDiagnosticsAsync() + { + await new VerifyCS.Test + { + TestCode = @" +#nullable enable + +using System; + +namespace Test +{ + public static class Test + { + public static void Method1() + { + // default + var objPlayers = new object?[] { null }; + + for (var idx = 0; idx < 1; idx++) + { + objPlayers[idx] = $""{idx}""; + } + + // validate + if (objPlayers[0] is null) // CA1508: Always true (should probably be unknown) + { + } + } + + public static void Method2(object?[] objPlayers, int idx) + { + objPlayers[0] = null; + objPlayers[idx] = 0; + if (objPlayers[0] is null) // CA1508: Always true (should probably be unknown) + { + } + } + } +}", + LanguageVersion = CodeAnalysis.CSharp.LanguageVersion.CSharp8, + }.RunAsync(); + } + + [Trait(Traits.DataflowAnalysis, Traits.Dataflow.NullAnalysis)] + [Trait(Traits.DataflowAnalysis, Traits.Dataflow.PointsToAnalysis)] + [Fact, WorkItem(6616, "https://github.com/dotnet/roslyn-analyzers/issues/6616")] + public async Task IndexedArrayAccessWithConstantsAndNonConstants_DiagnosticsAsync() + { + await new VerifyCS.Test + { + TestCode = @" +#nullable enable + +using System; + +namespace Test +{ + public static class Test + { + public static void Method1(object?[][] objPlayers, int idx) + { + objPlayers[0][0] = null; + + for (idx = 0; idx < 1; idx++) + { + objPlayers[1][idx] = 0; + } + + if (objPlayers[0][0] is null) // CA1508: Always true + { + } + } + + public static void Method2(object?[][] objPlayers, int idx) + { + objPlayers[0][0] = null; + objPlayers[1][idx] = 0; + if (objPlayers[0][0] is null) // CA1508: Always true + { + } + } + } +}", + LanguageVersion = CodeAnalysis.CSharp.LanguageVersion.CSharp8, + ExpectedDiagnostics = + { + // /0/Test0.cs(19,17): warning CA1508: 'objPlayers[0][0] is null' is always 'true'. Remove or refactor the condition(s) to avoid dead code. + GetCSharpResultAt(19, 17, "objPlayers[0][0] is null", "true"), + // /0/Test0.cs(28,17): warning CA1508: 'objPlayers[0][0] is null' is always 'true'. Remove or refactor the condition(s) to avoid dead code. + GetCSharpResultAt(28, 17, "objPlayers[0][0] is null", "true"), + }, + }.RunAsync(); + } + [Trait(Traits.DataflowAnalysis, Traits.Dataflow.NullAnalysis)] [Fact] public async Task ArrayInitializerNotParentedByArrayCreationAsync() diff --git a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/CopyAnalysis/CopyAnalysis.cs b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/CopyAnalysis/CopyAnalysis.cs index 0a50c36812..794b9e6849 100644 --- a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/CopyAnalysis/CopyAnalysis.cs +++ b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/CopyAnalysis/CopyAnalysis.cs @@ -16,6 +16,8 @@ namespace Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.CopyAnalysis /// public partial class CopyAnalysis : ForwardDataFlowAnalysis { + internal static readonly AbstractValueDomain ValueDomainInstance = CopyAbstractValueDomain.Default; + private CopyAnalysis(CopyDataFlowOperationVisitor operationVisitor) : base(operationVisitor.AnalysisDomain, operationVisitor) { @@ -43,7 +45,7 @@ private CopyAnalysis(CopyDataFlowOperationVisitor operationVisitor) wellKnownTypeProvider, pointsToAnalysisKind, interproceduralAnalysisConfig, interproceduralAnalysisPredicate, pessimisticAnalysis, performCopyAnalysis: false, exceptionPathsAnalysis) : null; - var analysisContext = CopyAnalysisContext.Create(CopyAbstractValueDomain.Default, wellKnownTypeProvider, + var analysisContext = CopyAnalysisContext.Create(ValueDomainInstance, wellKnownTypeProvider, cfg, owningSymbol, analyzerOptions, interproceduralAnalysisConfig, pessimisticAnalysis, exceptionPathsAnalysis, pointsToAnalysisResult, TryGetOrComputeResultForAnalysisContext, interproceduralAnalysisPredicate); return TryGetOrComputeResultForAnalysisContext(analysisContext); diff --git a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/CopyAnalysis/CopyAnalysisData.cs b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/CopyAnalysis/CopyAnalysisData.cs index be601582e5..606e7a97b0 100644 --- a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/CopyAnalysis/CopyAnalysisData.cs +++ b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/CopyAnalysis/CopyAnalysisData.cs @@ -7,6 +7,7 @@ namespace Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.CopyAnalysis { + using static Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.CopyAnalysis.CopyAnalysis; using CoreCopyAnalysisData = DictionaryAnalysisData; /// @@ -38,6 +39,7 @@ private CopyAnalysisData(CopyAnalysisData data1, CopyAnalysisData data2, MapAbst AssertValidCopyAnalysisData(); } + protected override AbstractValueDomain ValueDomain => ValueDomainInstance; public override AnalysisEntityBasedPredicateAnalysisData Clone() => new CopyAnalysisData(this); public override int Compare(AnalysisEntityBasedPredicateAnalysisData other, MapAbstractDomain coreDataAnalysisDomain) diff --git a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/PointsToAnalysis.PointsToAnalysisDomain.cs b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/PointsToAnalysis.PointsToAnalysisDomain.cs index e1a005f192..f28e869614 100644 --- a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/PointsToAnalysis.PointsToAnalysisDomain.cs +++ b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/PointsToAnalysis.PointsToAnalysisDomain.cs @@ -14,7 +14,7 @@ public partial class PointsToAnalysis : ForwardDataFlowAnalysis { public PointsToAnalysisDomain(DefaultPointsToValueGenerator defaultPointsToValueGenerator) - : base(new CorePointsToAnalysisDataDomain(defaultPointsToValueGenerator, PointsToAbstractValueDomainInstance)) + : base(new CorePointsToAnalysisDataDomain(defaultPointsToValueGenerator, ValueDomainInstance)) { } diff --git a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/PointsToAnalysis.cs b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/PointsToAnalysis.cs index fb0de7c2a8..413bcec913 100644 --- a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/PointsToAnalysis.cs +++ b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/PointsToAnalysis.cs @@ -17,7 +17,7 @@ namespace Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.PointsToAnalysis /// public partial class PointsToAnalysis : ForwardDataFlowAnalysis { - internal static readonly AbstractValueDomain PointsToAbstractValueDomainInstance = PointsToAbstractValueDomain.Default; + internal static readonly AbstractValueDomain ValueDomainInstance = PointsToAbstractValueDomain.Default; private PointsToAnalysis(PointsToAnalysisDomain analysisDomain, PointsToDataFlowOperationVisitor operationVisitor) : base(analysisDomain, operationVisitor) diff --git a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/PointsToAnalysisData.cs b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/PointsToAnalysisData.cs index 9478c7a152..2a1410ea01 100644 --- a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/PointsToAnalysisData.cs +++ b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/PointsToAnalysisData.cs @@ -51,6 +51,7 @@ private PointsToAnalysisData(PointsToAnalysisData data1, PointsToAnalysisData da AssertValidPointsToAnalysisData(); } + protected override AbstractValueDomain ValueDomain => PointsToAnalysis.ValueDomainInstance; public override AnalysisEntityBasedPredicateAnalysisData Clone() => new PointsToAnalysisData(this); public override int Compare(AnalysisEntityBasedPredicateAnalysisData other, MapAbstractDomain coreDataAnalysisDomain) diff --git a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/TaintedDataAnalysis/TaintedDataAnalysis.cs b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/TaintedDataAnalysis/TaintedDataAnalysis.cs index 3ee2438390..b9e827fcfd 100644 --- a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/TaintedDataAnalysis/TaintedDataAnalysis.cs +++ b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/TaintedDataAnalysis/TaintedDataAnalysis.cs @@ -16,6 +16,8 @@ namespace Analyzer.Utilities.FlowAnalysis.Analysis.TaintedDataAnalysis internal partial class TaintedDataAnalysis : ForwardDataFlowAnalysis { + internal static readonly AbstractValueDomain ValueDomainInstance = TaintedDataAbstractValueDomain.Default; + private TaintedDataAnalysis(TaintedDataAnalysisDomain analysisDomain, TaintedDataOperationVisitor operationVisitor) : base(analysisDomain, operationVisitor) { @@ -94,7 +96,7 @@ private TaintedDataAnalysis(TaintedDataAnalysisDomain analysisDomain, TaintedDat } TaintedDataAnalysisContext analysisContext = TaintedDataAnalysisContext.Create( - TaintedDataAbstractValueDomain.Default, + ValueDomainInstance, wellKnownTypeProvider, cfg, containingMethod, diff --git a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/TaintedDataAnalysis/TaintedDataAnalysisData.cs b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/TaintedDataAnalysis/TaintedDataAnalysisData.cs index 232776a2ab..3ecd8ecc4b 100644 --- a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/TaintedDataAnalysis/TaintedDataAnalysisData.cs +++ b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/TaintedDataAnalysis/TaintedDataAnalysisData.cs @@ -29,6 +29,7 @@ public TaintedDataAnalysisData(TaintedDataAnalysisData fromData, TaintedDataAnal { } + protected override AbstractValueDomain ValueDomain => TaintedDataAnalysis.ValueDomainInstance; public override AnalysisEntityBasedPredicateAnalysisData Clone() { return new TaintedDataAnalysisData(this); diff --git a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/ValueContentAnalysis/ValueContentAnalysis.cs b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/ValueContentAnalysis/ValueContentAnalysis.cs index 9853750b77..69c735688f 100644 --- a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/ValueContentAnalysis/ValueContentAnalysis.cs +++ b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/ValueContentAnalysis/ValueContentAnalysis.cs @@ -17,6 +17,8 @@ namespace Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.ValueContentAnalysis /// public partial class ValueContentAnalysis : ForwardDataFlowAnalysis { + internal static readonly AbstractValueDomain ValueDomainInstance = ValueContentAbstractValueDomain.Default; + private ValueContentAnalysis(ValueContentAnalysisDomain analysisDomain, ValueContentDataFlowOperationVisitor operationVisitor) : base(analysisDomain, operationVisitor) { @@ -98,7 +100,7 @@ private ValueContentAnalysis(ValueContentAnalysisDomain analysisDomain, ValueCon null; var analysisContext = ValueContentAnalysisContext.Create( - ValueContentAbstractValueDomain.Default, wellKnownTypeProvider, cfg, owningSymbol, analyzerOptions, + ValueDomainInstance, wellKnownTypeProvider, cfg, owningSymbol, analyzerOptions, interproceduralAnalysisConfig, pessimisticAnalysis, copyAnalysisResult, pointsToAnalysisResult, TryGetOrComputeResultForAnalysisContext, additionalSupportedValueTypes, getValueContentValueForAdditionalSupportedValueTypeOperation, diff --git a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/ValueContentAnalysis/ValueContentAnalysisData.cs b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/ValueContentAnalysis/ValueContentAnalysisData.cs index 7ec41dcb1c..9a6b851218 100644 --- a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/ValueContentAnalysis/ValueContentAnalysisData.cs +++ b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/ValueContentAnalysis/ValueContentAnalysisData.cs @@ -42,6 +42,7 @@ internal ValueContentAnalysisData( { } + protected override AbstractValueDomain ValueDomain => ValueContentAnalysis.ValueDomainInstance; public override AnalysisEntityBasedPredicateAnalysisData Clone() => new ValueContentAnalysisData(this); public override int Compare(AnalysisEntityBasedPredicateAnalysisData other, MapAbstractDomain coreDataAnalysisDomain) diff --git a/src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/AbstractIndex.cs b/src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/AbstractIndex.cs index 5f9d183b33..dc1e32f6eb 100644 --- a/src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/AbstractIndex.cs +++ b/src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/AbstractIndex.cs @@ -11,5 +11,7 @@ public abstract partial class AbstractIndex : CacheBasedEquatable public static AbstractIndex Create(int index) => new ConstantValueIndex(index); public static AbstractIndex Create(AnalysisEntity analysisEntity) => new AnalysisEntityBasedIndex(analysisEntity); public static AbstractIndex Create(IOperation operation) => new OperationBasedIndex(operation); + + internal bool IsConstant() => this is ConstantValueIndex; } } diff --git a/src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/AnalysisEntity.cs b/src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/AnalysisEntity.cs index 369f53013c..1793798eb2 100644 --- a/src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/AnalysisEntity.cs +++ b/src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/AnalysisEntity.cs @@ -120,7 +120,7 @@ public AnalysisEntity WithMergedInstanceLocation(AnalysisEntity analysisEntityTo Debug.Assert(EqualsIgnoringInstanceLocation(analysisEntityToMerge)); Debug.Assert(!InstanceLocation.Equals(analysisEntityToMerge.InstanceLocation)); - var mergedInstanceLocation = PointsToAnalysis.PointsToAnalysis.PointsToAbstractValueDomainInstance.Merge(InstanceLocation, analysisEntityToMerge.InstanceLocation); + var mergedInstanceLocation = PointsToAnalysis.PointsToAnalysis.ValueDomainInstance.Merge(InstanceLocation, analysisEntityToMerge.InstanceLocation); return new AnalysisEntity(Symbol, Indices, InstanceReferenceOperationSyntax, CaptureId, mergedInstanceLocation, Type, Parent, IsThisOrMeInstance); } @@ -204,6 +204,9 @@ or PointsToAbstractValueKind.UnknownNull public bool IsLValueFlowCaptureEntity => CaptureId.HasValue && CaptureId.Value.IsLValueFlowCapture; + internal AnalysisEntity WithIndices(ImmutableArray indices) + => new(Symbol, indices, InstanceReferenceOperationSyntax, CaptureId, InstanceLocation, Type, Parent, IsThisOrMeInstance); + public bool EqualsIgnoringInstanceLocation(AnalysisEntity? other) { // Perform fast equality checks first. diff --git a/src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/AnalysisEntityBasedPredicateAnalysisData.cs b/src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/AnalysisEntityBasedPredicateAnalysisData.cs index 815c5034f7..5061463415 100644 --- a/src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/AnalysisEntityBasedPredicateAnalysisData.cs +++ b/src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/AnalysisEntityBasedPredicateAnalysisData.cs @@ -5,6 +5,7 @@ using System.Collections.Immutable; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; +using System.Linq; namespace Microsoft.CodeAnalysis.FlowAnalysis.DataFlow { @@ -58,6 +59,7 @@ protected AnalysisEntityBasedPredicateAnalysisData( public virtual bool HasAnyAbstractValue => CoreAnalysisData.Count > 0 || HasPredicatedData; + protected abstract AbstractValueDomain ValueDomain { get; } public abstract AnalysisEntityBasedPredicateAnalysisData Clone(); public abstract AnalysisEntityBasedPredicateAnalysisData WithMergedData(AnalysisEntityBasedPredicateAnalysisData data, MapAbstractDomain coreDataAnalysisDomain); public abstract int Compare(AnalysisEntityBasedPredicateAnalysisData other, MapAbstractDomain coreDataAnalysisDomain); @@ -100,6 +102,8 @@ public virtual void SetAbstractValue(AnalysisEntity key, TValue value) } CoreAnalysisData[key] = value; + + ClearOverlappingAnalysisDataForIndexedEntity(key, value); } public void RemoveEntries(AnalysisEntity key) @@ -159,6 +163,46 @@ public PredicateValueKind ApplyPredicatedDataForEntity(AnalysisEntity predicated public void AddTrackedEntities(HashSet builder) => builder.UnionWith(CoreAnalysisData.Keys); + private void ClearOverlappingAnalysisDataForIndexedEntity(AnalysisEntity analysisEntity, TValue value) + { + if (!analysisEntity.Indices.Any(index => !index.IsConstant())) + { + return; + } + + foreach (var entity in CoreAnalysisData.Keys) + { + if (entity.Indices.Length != analysisEntity.Indices.Length || + entity == analysisEntity) + { + continue; + } + + var canOverlap = true; + for (var i = 0; i < entity.Indices.Length; i++) + { + if (entity.Indices[i].IsConstant() && + analysisEntity.Indices[i].IsConstant() && + !entity.Indices[i].Equals(analysisEntity.Indices[i])) + { + canOverlap = false; + break; + } + } + + if (canOverlap && + entity.WithIndices(analysisEntity.Indices).Equals(analysisEntity) && + CoreAnalysisData.TryGetValue(entity, out var existingValue)) + { + var mergedValue = ValueDomain.Merge(value, existingValue); + if (!existingValue!.Equals(mergedValue)) + { + SetAbstractValue(entity, mergedValue); + } + } + } + } + protected override void Dispose(bool disposing) { if (IsDisposed)