From 8e26f3b830e7c2e928aa3c3a3e590b9775972fe6 Mon Sep 17 00:00:00 2001 From: Benedikt Reinartz Date: Thu, 22 Aug 2024 12:43:14 +0200 Subject: [PATCH] Try to be strict around non-copyability --- Directory.Build.props | 8 +- pythonnet.sln | 23 ++ src/runtime/Native/NativeCall.cs | 6 +- src/runtime/Native/NewReference.cs | 2 +- src/runtime/Python.Runtime.csproj | 4 + src/runtime/PythonTypes/PyFloat.cs | 1 + src/runtime/PythonTypes/PyInt.cs | 1 + src/runtime/Runtime.cs | 12 +- src/runtime/Types/Iterator.cs | 2 +- src/runtime/Types/ManagedType.cs | 4 +- .../AnalyzerReleases.Shipped.md | 0 .../AnalyzerReleases.Unshipped.md | 5 + .../NonCopyableAnalyzer.cs | 293 ++++++++++++++++++ .../NonCopyableAnalyzer.csproj | 39 +++ tools/NonCopyableAnalyzer/TypeExtensions.cs | 112 +++++++ 15 files changed, 494 insertions(+), 18 deletions(-) create mode 100644 tools/NonCopyableAnalyzer/AnalyzerReleases.Shipped.md create mode 100644 tools/NonCopyableAnalyzer/AnalyzerReleases.Unshipped.md create mode 100644 tools/NonCopyableAnalyzer/NonCopyableAnalyzer.cs create mode 100644 tools/NonCopyableAnalyzer/NonCopyableAnalyzer.csproj create mode 100644 tools/NonCopyableAnalyzer/TypeExtensions.cs diff --git a/Directory.Build.props b/Directory.Build.props index e45c16f6a..72df6c9fe 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -11,14 +11,10 @@ $(FullVersion.Split('-', 2)[1]) - - + + all runtime; build; native; contentfiles; analyzers - - all - runtime; build; native; contentfiles; analyzers; buildtransitive - diff --git a/pythonnet.sln b/pythonnet.sln index d1a47892e..cc0f9893d 100644 --- a/pythonnet.sln +++ b/pythonnet.sln @@ -48,6 +48,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Build", "Build", "{142A6752 Directory.Build.props = Directory.Build.props EndProjectSection EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "NonCopyableAnalyzer", "tools\NonCopyableAnalyzer\NonCopyableAnalyzer.csproj", "{FE20DBEA-F4F9-4280-AD5E-82B82AE32DDF}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -187,6 +189,24 @@ Global {35CBBDEB-FC07-4D04-9D3E-F88FC180110B}.TraceAlloc|x64.Build.0 = Debug|Any CPU {35CBBDEB-FC07-4D04-9D3E-F88FC180110B}.TraceAlloc|x86.ActiveCfg = Debug|Any CPU {35CBBDEB-FC07-4D04-9D3E-F88FC180110B}.TraceAlloc|x86.Build.0 = Debug|Any CPU + {FE20DBEA-F4F9-4280-AD5E-82B82AE32DDF}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {FE20DBEA-F4F9-4280-AD5E-82B82AE32DDF}.Debug|Any CPU.Build.0 = Debug|Any CPU + {FE20DBEA-F4F9-4280-AD5E-82B82AE32DDF}.Debug|x64.ActiveCfg = Debug|Any CPU + {FE20DBEA-F4F9-4280-AD5E-82B82AE32DDF}.Debug|x64.Build.0 = Debug|Any CPU + {FE20DBEA-F4F9-4280-AD5E-82B82AE32DDF}.Debug|x86.ActiveCfg = Debug|Any CPU + {FE20DBEA-F4F9-4280-AD5E-82B82AE32DDF}.Debug|x86.Build.0 = Debug|Any CPU + {FE20DBEA-F4F9-4280-AD5E-82B82AE32DDF}.Release|Any CPU.ActiveCfg = Release|Any CPU + {FE20DBEA-F4F9-4280-AD5E-82B82AE32DDF}.Release|Any CPU.Build.0 = Release|Any CPU + {FE20DBEA-F4F9-4280-AD5E-82B82AE32DDF}.Release|x64.ActiveCfg = Release|Any CPU + {FE20DBEA-F4F9-4280-AD5E-82B82AE32DDF}.Release|x64.Build.0 = Release|Any CPU + {FE20DBEA-F4F9-4280-AD5E-82B82AE32DDF}.Release|x86.ActiveCfg = Release|Any CPU + {FE20DBEA-F4F9-4280-AD5E-82B82AE32DDF}.Release|x86.Build.0 = Release|Any CPU + {FE20DBEA-F4F9-4280-AD5E-82B82AE32DDF}.TraceAlloc|Any CPU.ActiveCfg = Debug|Any CPU + {FE20DBEA-F4F9-4280-AD5E-82B82AE32DDF}.TraceAlloc|Any CPU.Build.0 = Debug|Any CPU + {FE20DBEA-F4F9-4280-AD5E-82B82AE32DDF}.TraceAlloc|x64.ActiveCfg = Debug|Any CPU + {FE20DBEA-F4F9-4280-AD5E-82B82AE32DDF}.TraceAlloc|x64.Build.0 = Debug|Any CPU + {FE20DBEA-F4F9-4280-AD5E-82B82AE32DDF}.TraceAlloc|x86.ActiveCfg = Debug|Any CPU + {FE20DBEA-F4F9-4280-AD5E-82B82AE32DDF}.TraceAlloc|x86.Build.0 = Debug|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -194,4 +214,7 @@ Global GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {C8845072-C642-4858-8627-27E862AD21BB} EndGlobalSection + GlobalSection(NestedProjects) = preSolution + {FE20DBEA-F4F9-4280-AD5E-82B82AE32DDF} = {BC426F42-8494-4AA5-82C9-5109ACD97BD1} + EndGlobalSection EndGlobal diff --git a/src/runtime/Native/NativeCall.cs b/src/runtime/Native/NativeCall.cs index 2ea7b344e..f14465ba5 100644 --- a/src/runtime/Native/NativeCall.cs +++ b/src/runtime/Native/NativeCall.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics.CodeAnalysis; namespace Python.Runtime { @@ -9,12 +10,13 @@ namespace Python.Runtime /// situations (specifically, calling functions through Python /// type structures) where we need to call functions indirectly. /// + [ExcludeFromCodeCoverage] internal unsafe class NativeCall { public static void CallDealloc(IntPtr fp, StolenReference a1) { - var d = (delegate* unmanaged[Cdecl])fp; - d(a1); + var d = (delegate* unmanaged[Cdecl])fp; + d(a1.DangerousGetAddress()); } public static NewReference Call_3(IntPtr fp, BorrowedReference a1, BorrowedReference a2, BorrowedReference a3) diff --git a/src/runtime/Native/NewReference.cs b/src/runtime/Native/NewReference.cs index f7a030818..eeef86e85 100644 --- a/src/runtime/Native/NewReference.cs +++ b/src/runtime/Native/NewReference.cs @@ -124,7 +124,7 @@ public void Dispose() /// [Pure] public static NewReference DangerousFromPointer(IntPtr pointer) - => new() { pointer = pointer}; + => new NewReference { pointer = pointer }; [Pure] internal static IntPtr DangerousGetAddressOrNull(in NewReference reference) => reference.pointer; diff --git a/src/runtime/Python.Runtime.csproj b/src/runtime/Python.Runtime.csproj index 5072f23cd..4a44ac1ea 100644 --- a/src/runtime/Python.Runtime.csproj +++ b/src/runtime/Python.Runtime.csproj @@ -65,5 +65,9 @@ + + false + Analyzer + diff --git a/src/runtime/PythonTypes/PyFloat.cs b/src/runtime/PythonTypes/PyFloat.cs index 50621d5c2..ff4c2b6b7 100644 --- a/src/runtime/PythonTypes/PyFloat.cs +++ b/src/runtime/PythonTypes/PyFloat.cs @@ -103,5 +103,6 @@ public static PyFloat AsFloat(PyObject value) public double ToDouble() => Runtime.PyFloat_AsDouble(obj); public override TypeCode GetTypeCode() => TypeCode.Double; + public override int GetHashCode() => ((PyObject)this).GetHashCode(); } } diff --git a/src/runtime/PythonTypes/PyInt.cs b/src/runtime/PythonTypes/PyInt.cs index 0d00f5a13..b4d7195ec 100644 --- a/src/runtime/PythonTypes/PyInt.cs +++ b/src/runtime/PythonTypes/PyInt.cs @@ -232,5 +232,6 @@ public string ToString(string format, IFormatProvider formatProvider) } public override TypeCode GetTypeCode() => TypeCode.Int64; + public override int GetHashCode() => ((PyObject)this).GetHashCode(); } } diff --git a/src/runtime/Runtime.cs b/src/runtime/Runtime.cs index a33e20b4a..3f73f362d 100644 --- a/src/runtime/Runtime.cs +++ b/src/runtime/Runtime.cs @@ -678,7 +678,7 @@ internal static T TryUsingDll(Func op) /// /// PyObject Ptr - internal static void Py_DecRef(StolenReference ob) => Delegates.Py_DecRef(ob); + internal static void Py_DecRef(StolenReference ob) => Delegates.Py_DecRef(ob.AnalyzerWorkaround()); internal static void Py_Initialize() => Delegates.Py_Initialize(); @@ -1459,7 +1459,7 @@ internal static bool PyList_Check(BorrowedReference ob) internal static BorrowedReference PyList_GetItem(BorrowedReference pointer, nint index) => Delegates.PyList_GetItem(pointer, index); - internal static int PyList_SetItem(BorrowedReference pointer, nint index, StolenReference value) => Delegates.PyList_SetItem(pointer, index, value); + internal static int PyList_SetItem(BorrowedReference pointer, nint index, StolenReference value) => Delegates.PyList_SetItem(pointer, index, value.AnalyzerWorkaround()); internal static int PyList_Insert(BorrowedReference pointer, nint index, BorrowedReference value) => Delegates.PyList_Insert(pointer, index, value); @@ -1497,7 +1497,7 @@ internal static int PyTuple_SetItem(BorrowedReference pointer, nint index, Borro return PyTuple_SetItem(pointer, index, newRef.Steal()); } - internal static int PyTuple_SetItem(BorrowedReference pointer, nint index, StolenReference value) => Delegates.PyTuple_SetItem(pointer, index, value); + internal static int PyTuple_SetItem(BorrowedReference pointer, nint index, StolenReference value) => Delegates.PyTuple_SetItem(pointer, index, value.AnalyzerWorkaround()); internal static NewReference PyTuple_GetSlice(BorrowedReference pointer, nint start, nint end) => Delegates.PyTuple_GetSlice(pointer, start, end); @@ -1657,7 +1657,7 @@ internal static bool PyType_IsSameAsOrSubtype(BorrowedReference type, BorrowedRe internal static NewReference PyObject_GenericGetDict(BorrowedReference o) => PyObject_GenericGetDict(o, IntPtr.Zero); internal static NewReference PyObject_GenericGetDict(BorrowedReference o, IntPtr context) => Delegates.PyObject_GenericGetDict(o, context); - internal static void PyObject_GC_Del(StolenReference ob) => Delegates.PyObject_GC_Del(ob); + internal static void PyObject_GC_Del(StolenReference ob) => Delegates.PyObject_GC_Del(ob.AnalyzerWorkaround()); internal static bool PyObject_GC_IsTracked(BorrowedReference ob) @@ -1720,7 +1720,7 @@ internal static void PyErr_SetString(BorrowedReference ob, string message) internal static void PyErr_Fetch(out NewReference type, out NewReference val, out NewReference tb) => Delegates.PyErr_Fetch(out type, out val, out tb); - internal static void PyErr_Restore(StolenReference type, StolenReference val, StolenReference tb) => Delegates.PyErr_Restore(type, val, tb); + internal static void PyErr_Restore(StolenReference type, StolenReference val, StolenReference tb) => Delegates.PyErr_Restore(type.AnalyzerWorkaround(), val.AnalyzerWorkaround(), tb.AnalyzerWorkaround()); internal static void PyErr_Clear() => Delegates.PyErr_Clear(); @@ -1738,7 +1738,7 @@ internal static NewReference PyException_GetTraceback(BorrowedReference ex) /// Set the cause associated with the exception to cause. Use NULL to clear it. There is no type check to make sure that cause is either an exception instance or None. This steals a reference to cause. /// internal static void PyException_SetCause(BorrowedReference ex, StolenReference cause) - => Delegates.PyException_SetCause(ex, cause); + => Delegates.PyException_SetCause(ex, cause.AnalyzerWorkaround()); internal static int PyException_SetTraceback(BorrowedReference ex, BorrowedReference tb) => Delegates.PyException_SetTraceback(ex, tb); diff --git a/src/runtime/Types/Iterator.cs b/src/runtime/Types/Iterator.cs index 49145d2c3..defa7e9f4 100644 --- a/src/runtime/Types/Iterator.cs +++ b/src/runtime/Types/Iterator.cs @@ -46,6 +46,6 @@ public static NewReference tp_iternext(BorrowedReference ob) return Converter.ToPython(item, self.elemType); } - public static NewReference tp_iter(BorrowedReference ob) => new (ob); + public static NewReference tp_iter(BorrowedReference ob) => new NewReference(ob); } } diff --git a/src/runtime/Types/ManagedType.cs b/src/runtime/Types/ManagedType.cs index 97a19497c..e229c0f8e 100644 --- a/src/runtime/Types/ManagedType.cs +++ b/src/runtime/Types/ManagedType.cs @@ -83,8 +83,8 @@ internal static unsafe void DecrefTypeAndFree(StolenReference ob) var freePtr = Util.ReadIntPtr(type, TypeOffset.tp_free); Debug.Assert(freePtr != IntPtr.Zero); - var free = (delegate* unmanaged[Cdecl])freePtr; - free(ob); + var free = (delegate* unmanaged[Cdecl])freePtr; + free(ref ob); Runtime.XDecref(StolenReference.DangerousFromPointer(type.DangerousGetAddress())); } diff --git a/tools/NonCopyableAnalyzer/AnalyzerReleases.Shipped.md b/tools/NonCopyableAnalyzer/AnalyzerReleases.Shipped.md new file mode 100644 index 000000000..e69de29bb diff --git a/tools/NonCopyableAnalyzer/AnalyzerReleases.Unshipped.md b/tools/NonCopyableAnalyzer/AnalyzerReleases.Unshipped.md new file mode 100644 index 000000000..1d6de40ab --- /dev/null +++ b/tools/NonCopyableAnalyzer/AnalyzerReleases.Unshipped.md @@ -0,0 +1,5 @@ +### New Rules + +Rule ID | Category | Severity | Notes +--------|----------|----------|-------------------- +NoCopy99|Correction| Warning | N/A diff --git a/tools/NonCopyableAnalyzer/NonCopyableAnalyzer.cs b/tools/NonCopyableAnalyzer/NonCopyableAnalyzer.cs new file mode 100644 index 000000000..9618ae524 --- /dev/null +++ b/tools/NonCopyableAnalyzer/NonCopyableAnalyzer.cs @@ -0,0 +1,293 @@ +using System; +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace NonCopyable +{ + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public class NonCopyableAnalyzer : DiagnosticAnalyzer + { + private static DiagnosticDescriptor CreateRule(int num, string type) + => new DiagnosticDescriptor("NoCopy" + num.ToString("00"), "non-copyable", "🚫 " + type + ". '{0}' is non-copyable.", "Design", DiagnosticSeverity.Error, isEnabledByDefault: true); + + private static DiagnosticDescriptor FieldDeclarationRule = CreateRule(1, "field declaration"); + private static DiagnosticDescriptor InitializerRule = CreateRule(2, "initializer"); + private static DiagnosticDescriptor AssignmentRule = CreateRule(3, "assignment"); + private static DiagnosticDescriptor ArgumentRule = CreateRule(4, "argument"); + private static DiagnosticDescriptor ReturnRule = CreateRule(5, "return"); + private static DiagnosticDescriptor ConversionRule = CreateRule(6, "conversion"); + private static DiagnosticDescriptor PatternRule = CreateRule(7, "pattern matching"); + private static DiagnosticDescriptor TupleRule = CreateRule(8, "tuple"); + private static DiagnosticDescriptor MemberRule = CreateRule(9, "member reference"); + private static DiagnosticDescriptor ReadOnlyInvokeRule = CreateRule(10, "readonly invoke"); + private static DiagnosticDescriptor GenericConstraintRule = CreateRule(11, "generic constraint"); + private static DiagnosticDescriptor DelegateRule = CreateRule(12, "delegate"); + + private static DiagnosticDescriptor InfoRule = new("NoCopy99", "non-copyable", "info: {0}", "Correction", DiagnosticSeverity.Warning, true); + + public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(FieldDeclarationRule, InitializerRule, AssignmentRule, ArgumentRule, ReturnRule, ConversionRule, PatternRule, TupleRule, MemberRule, ReadOnlyInvokeRule, GenericConstraintRule, DelegateRule, InfoRule); + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + context.RegisterCompilationStartAction(csc => + { + csc.RegisterOperationAction(oc => + { + var op = (ISymbolInitializerOperation)oc.Operation; + CheckCopyability(oc, op.Value, InitializerRule); + }, OperationKind.FieldInitializer, + OperationKind.ParameterInitializer, + OperationKind.PropertyInitializer, + OperationKind.VariableInitializer); + + csc.RegisterOperationAction(oc => + { + // including member initializer + // including collection element initializer + var op = (ISimpleAssignmentOperation)oc.Operation; + if (op.IsRef) return; + CheckCopyability(oc, op.Value, AssignmentRule); + }, OperationKind.SimpleAssignment); + + csc.RegisterOperationAction(oc => + { + // including non-ref extension method invocation + var op = (IArgumentOperation)oc.Operation; + if (op.Parameter.RefKind != RefKind.None) return; + CheckCopyability(oc, op.Value, ArgumentRule); + }, OperationKind.Argument); + + csc.RegisterOperationAction(oc => + { + var op = (IReturnOperation)oc.Operation; + + if (op.ReturnedValue == null) return; + + // In the abscense of data flow analysis we can treat + // return of a local variable or a parameter as a "move". + if ((op.ReturnedValue.Kind == OperationKind.LocalReference + || op.ReturnedValue.Kind == OperationKind.ParameterReference) + && op.Kind == OperationKind.Return) + { + var varScope = op.ReturnedValue.GetSymbol().ContainingSymbol; + var opScope = op.SemanticModel.GetEnclosingSymbol(op.Syntax.SpanStart); + if (SymbolEqualityComparer.Default.Equals(varScope, opScope)) return; + } + + if (( + op.ReturnedValue.Kind == OperationKind.Invocation + || op.ReturnedValue.Kind == OperationKind.FunctionPointerInvocation + || op.ReturnedValue.Kind == OperationKind.DynamicInvocation + || op.ReturnedValue.Kind == OperationKind.ObjectCreation + || op.ReturnedValue.Kind == OperationKind.DynamicObjectCreation + || op.ReturnedValue.Kind == OperationKind.Conversion + ) && op.Kind == OperationKind.Return) + return; + + if (!CheckCopyability(oc, op.ReturnedValue, ReturnRule)) + { + oc.ReportDiagnostic(Error(op.Syntax, InfoRule, $"🚫 {op.ReturnedValue}, {op.ReturnedValue.Kind}")); + } + }, OperationKind.Return, + OperationKind.YieldReturn); + + csc.RegisterOperationAction(oc => + { + var op = (IConversionOperation)oc.Operation; + var v = op.Operand; + if (v.Kind == OperationKind.DefaultValue) return; + var t = v.Type; + if (!t.IsNonCopyable()) return; + + if (op.OperatorMethod != null && op.OperatorMethod.Parameters.Length == 1) + { + var parameter = op.OperatorMethod.Parameters[0]; + if (parameter.RefKind != RefKind.None) return; + } + + if (op.Parent is IForEachLoopOperation && + op == ((IForEachLoopOperation)op.Parent).Collection && + op.Conversion.IsIdentity) + { + return; + } + + oc.ReportDiagnostic(Error(v.Syntax, ConversionRule, t.Name)); + }, OperationKind.Conversion); + + csc.RegisterOperationAction(oc => + { + var op = (IArrayInitializerOperation)oc.Operation; + + if (!((IArrayTypeSymbol)((IArrayCreationOperation)op.Parent).Type).ElementType.IsNonCopyable()) return; + + foreach (var v in op.ElementValues) + { + CheckCopyability(oc, v, InitializerRule); + } + }, OperationKind.ArrayInitializer); + + csc.RegisterOperationAction(oc => + { + var op = (IDeclarationPatternOperation)oc.Operation; + + if (op.DeclaredSymbol is ILocalSymbol t) + { + if (!t.Type.IsNonCopyable()) + return; + else + oc.ReportDiagnostic(Error(op.Syntax, PatternRule, t.Name)); + } + }, OperationKind.DeclarationPattern); + + csc.RegisterOperationAction(oc => + { + var op = (ITupleOperation)oc.Operation; + + // exclude ParenthesizedVariableDesignationSyntax + if (!op.Syntax.IsKind(SyntaxKind.TupleExpression)) return; + + foreach (var v in op.Elements) + { + CheckCopyability(oc, v, TupleRule); + } + }, OperationKind.Tuple); + + csc.RegisterOperationAction(oc => + { + // instance property/event should not be referenced with in parameter/ref readonly local/readonly field + var op = (IMemberReferenceOperation)oc.Operation; + CheckInstanceReadonly(oc, op.Instance, MemberRule); + }, OperationKind.PropertyReference, + OperationKind.EventReference); + + csc.RegisterOperationAction(oc => + { + // instance method should not be invoked with in parameter/ref readonly local/readonly field + var op = (IInvocationOperation)oc.Operation; + + CheckGenericConstraints(oc, op, GenericConstraintRule); + CheckInstanceReadonly(oc, op.Instance, ReadOnlyInvokeRule); + + }, OperationKind.Invocation); + + csc.RegisterOperationAction(oc => { + var op = (IDynamicInvocationOperation)oc.Operation; + + foreach(var arg in op.Arguments) { + if (!arg.Type.IsNonCopyable()) continue; + + oc.ReportDiagnostic(Error(arg.Syntax, GenericConstraintRule)); + } + + }, OperationKind.DynamicInvocation); + + csc.RegisterOperationAction(oc => + { + // delagate creation + var op = (IMemberReferenceOperation)oc.Operation; + if (op.Instance == null) return; + if (!op.Instance.Type.IsNonCopyable()) return; + oc.ReportDiagnostic(Error(op.Instance.Syntax, DelegateRule, op.Instance.Type.Name)); + }, OperationKind.MethodReference); + + csc.RegisterSymbolAction(sac => + { + var f = (IFieldSymbol)sac.Symbol; + if (f.IsStatic) return; + if (!f.Type.IsNonCopyable()) return; + if (f.ContainingType.IsReferenceType) return; + if (f.ContainingType.IsNonCopyable()) return; + sac.ReportDiagnostic(Error(f.DeclaringSyntaxReferences[0].GetSyntax(), FieldDeclarationRule, f.Type.Name)); + }, SymbolKind.Field); + }); + + // not supported yet: + // OperationKind.CompoundAssignment, + // OperationKind.UnaryOperator, + // OperationKind.BinaryOperator, + } + + private static void CheckGenericConstraints(in OperationAnalysisContext oc, IInvocationOperation op, DiagnosticDescriptor rule) + { + var m = op.TargetMethod; + + if (m.IsGenericMethod) + { + var parameters = m.TypeParameters; + var arguments = m.TypeArguments; + for (int i = 0; i < parameters.Length; i++) + { + var p = parameters[i]; + var a = arguments[i]; + + if (a.IsNonCopyable() && !p.IsNonCopyable()) + oc.ReportDiagnostic(Error(op.Syntax, rule, a.Name)); + } + } + } + + private static void CheckInstanceReadonly(in OperationAnalysisContext oc, IOperation instance, DiagnosticDescriptor rule) + { + if (instance == null) return; + + var t = instance.Type; + if (!t.IsNonCopyable()) return; + + if (IsInstanceReadonly(instance)) + { + oc.ReportDiagnostic(Error(instance.Syntax, rule, t.Name)); + } + } + + private static Diagnostic Error(SyntaxNode at, DiagnosticDescriptor rule, string name = null) + => name is null + ? Diagnostic.Create(rule, at.GetLocation()) + : Diagnostic.Create(rule, at.GetLocation(), name); + + private static bool IsInstanceReadonly(IOperation instance) + { + bool isReadOnly = false; + switch (instance) + { + case IFieldReferenceOperation r: + isReadOnly = r.Field.IsReadOnly; + break; + case ILocalReferenceOperation r: + isReadOnly = r.Local.RefKind == RefKind.In; + break; + case IParameterReferenceOperation r: + isReadOnly = r.Parameter.RefKind == RefKind.In; + break; + } + + return isReadOnly; + } + + private static bool HasNonCopyableParameter(IMethodSymbol m) + { + foreach (var p in m.Parameters) + { + if(p.RefKind == RefKind.None) + { + if (p.Type.IsNonCopyable()) return true; + } + } + return false; + } + + private static bool CheckCopyability(in OperationAnalysisContext oc, IOperation v, DiagnosticDescriptor rule) + { + var t = v.Type; + if (!t.IsNonCopyable()) return true; + if (v.CanCopy()) return true; + oc.ReportDiagnostic(Error(v.Syntax, rule, t.Name)); + return false; + } + } +} diff --git a/tools/NonCopyableAnalyzer/NonCopyableAnalyzer.csproj b/tools/NonCopyableAnalyzer/NonCopyableAnalyzer.csproj new file mode 100644 index 000000000..cb09fa270 --- /dev/null +++ b/tools/NonCopyableAnalyzer/NonCopyableAnalyzer.csproj @@ -0,0 +1,39 @@ + + + + netstandard2.0 + false + True + 9.0 + + + + NonCopyableAnalyzer + 0.7.0 + Nobuyuki Iwanaga + https://github.com/ufcpp/NonCopyableAnalyzer/blob/master/LICENSE + https://github.com/ufcpp/NonCopyableAnalyzer + false + Analyzer for Non-copyable struct + Support for C# 9 unmanaged function pointers. +Allow usage of non-copyable in conditional expressions (? :). +Treat return statement as having move semantics for locals and parameters. + NonCopyable, analyzers + true + + + + true + + + + + + + + + + + + + diff --git a/tools/NonCopyableAnalyzer/TypeExtensions.cs b/tools/NonCopyableAnalyzer/TypeExtensions.cs new file mode 100644 index 000000000..2946f8e62 --- /dev/null +++ b/tools/NonCopyableAnalyzer/TypeExtensions.cs @@ -0,0 +1,112 @@ +using System; + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Operations; + +namespace NonCopyable +{ + public static class TypeExtensions + { + public static bool IsNonCopyable(this ITypeSymbol t) + { + if (t == null) return false; + if (t.TypeKind != TypeKind.Struct && t.TypeKind != TypeKind.TypeParameter) return false; + + if (HasNonCopyableAttribute(t)) return true; + + if (t.TypeKind == TypeKind.Struct) + { + foreach (var ifType in t.AllInterfaces) + { + if (HasNonCopyableAttribute(ifType)) return true; + } + } + else + { + foreach (var constraint in ((ITypeParameterSymbol)t).ConstraintTypes) + { + if (HasNonCopyableAttribute(constraint)) return true; + } + + } + + return false; + } + + private static bool HasNonCopyableAttribute(ITypeSymbol t) + { + foreach (var a in t.GetAttributes()) + { + var str = a.AttributeClass.Name; + if (str.EndsWith("NonCopyable") || str.EndsWith("NonCopyableAttribute")) return true; + } + + return false; + } + + private static SyntaxList GetAttributes(this SyntaxNode syntax) + { + switch (syntax) + { + case StructDeclarationSyntax s: + return s.AttributeLists; + case TypeParameterSyntax s: + return s.AttributeLists; + default: + return default; + } + } + + /// + /// test whether op is copyable or not even when it is a non-copyable instance. + /// + public static bool CanCopy(this IOperation op) + { + var k = op.Kind; + + if (k == OperationKind.Conversion) + { + var operandKind = ((IConversionOperation)op).Operand.Kind; + // default literal (invalid if LangVersion < 7.1) + if (operandKind == OperationKind.DefaultValue || operandKind == OperationKind.Invalid) return true; + } + + if (k == OperationKind.LocalReference || k == OperationKind.FieldReference || k == OperationKind.PropertyReference || k == OperationKind.ArrayElementReference) + { + //need help: how to get ref-ness from IOperation? + var parent = op.Syntax.Parent.Kind(); + if (parent == SyntaxKind.RefExpression) return true; + } + + if (k == OperationKind.Conditional) + { + var cond = (IConditionalOperation)op; + return cond.WhenFalse.CanCopy() && cond.WhenFalse.CanCopy(); + } + + return k == OperationKind.ObjectCreation + || k == OperationKind.DefaultValue + || k == OperationKind.Literal + || k == OperationKind.Invocation + // workaround for https://github.com/dotnet/roslyn/issues/49751 + || !IsValid(k) && op.Syntax is InvocationExpressionSyntax; + + //todo: should return value be OK? + //todo: move semantics + } + + static bool IsValid(OperationKind kind) + => kind != OperationKind.None && kind != OperationKind.Invalid; + + public static ISymbol GetSymbol(this IOperation op) + { + if (op.Kind == OperationKind.LocalReference) + return ((ILocalReferenceOperation)op).Local; + if (op.Kind == OperationKind.ParameterReference) + return ((IParameterReferenceOperation)op).Parameter; + throw new NotSupportedException(op.Kind.ToString()); + } + } +}