diff --git a/.claude/rules/netstandard21-compatibility.md b/.claude/rules/netstandard21-compatibility.md index b6570968..27352bcf 100644 --- a/.claude/rules/netstandard21-compatibility.md +++ b/.claude/rules/netstandard21-compatibility.md @@ -23,6 +23,7 @@ Some `Microsoft.Dynamics.Nav.CodeAnalysis` APIs only exist in newer SDK versions |---|---|---| | `IFieldSymbol.Type` | net8.0 only | `fieldSymbol.OriginalDefinition.GetTypeSymbol()` (requires `using Microsoft.Dynamics.Nav.CodeAnalysis.Symbols`) | | `ThisExpressionSyntax`, `SyntaxKind.ThisExpression`, `IInstanceReferenceOperation` (AL `this`) | net8.0+ only | **Do not reference them at all** (a guard would silently drop `this` handling on the netstandard2.1 binary that serves AL 14.0 to 15.2). Resolve the receiver through the operation tree, which exists at the floor: `.claude/rules/receiver-forms.md`. | +| `IMethodSymbol.IsStatic` | net8.0+ (AL 16+) | `TargetMethod.ContainingSymbol is IClassTypeSymbol` whose name is not `"Table"` identifies a static built-in; see `GetReceiverTableType` in `OperationExtensions.cs`. | ### Pattern for `IFieldSymbol.Type` diff --git a/.claude/rules/receiver-forms.md b/.claude/rules/receiver-forms.md index 321b8859..983b5105 100644 --- a/.claude/rules/receiver-forms.md +++ b/.claude/rules/receiver-forms.md @@ -15,10 +15,10 @@ Any rule that looks at a record field (`Rec."No."`), a record method (`Modify`, |---|---|---|---|---| | Named variable | `Customer.Modify()`, `Customer."No."` | `MemberAccessExpressionSyntax` with an `IdentifierNameSyntax` receiver | `Instance` is a variable or parameter reference | variable map, or `Instance.GetSymbolSafe()` then `IVariableSymbol.Type` | | Implicit `Rec` | `Rec.Modify()`, `Rec."No."` | same (`Rec` is an ordinary identifier) | `Instance` references the synthesized `Rec`: a global on most objects, a local in a `TableNo` codeunit (next section) | same as named variable | -| Bare self | `Modify()`, `"No."` | `InvocationExpressionSyntax` or `IdentifierNameSyntax` without a receiver | inside tables and tableextensions `Instance` is **null**; elsewhere the binder inserts the `Rec` receiver | the containing object for a null instance; otherwise `Instance.Type` | +| Bare self | `Modify()`, `"No."` | `InvocationExpressionSyntax` or `IdentifierNameSyntax` without a receiver | inside tables and tableextensions `Instance` is **null** (ambiguous: also null for any `IsStatic` target — `IsolatedStorage.Get`, `NumberSequence.Next`, bare `Error`, `Format`); elsewhere the binder inserts the `Rec` receiver | the containing object for a null instance; otherwise `Instance.Type` | | `this` (runtime 14.0+) | `this.Modify()`, `this."No."` | `MemberAccessExpressionSyntax` whose receiver is not an `IdentifierNameSyntax` | `Instance.Kind == OperationKind.ThisReference`; `Instance.Type` is the record type named after the table | `Instance.Type`, or `SemanticModel.GetOperation(receiver)?.Type` | -The forms apply identically to `IInvocationExpression.Instance` (built-in record methods and user procedures: `MyProc()`, `this.MyProc()`, `Rec.MyProc()`) and to `IFieldAccess.Instance`. `GetReceiverTableType` in `ALCops.Common/Extensions/OperationExtensions.cs` is the canonical resolver for both, including the null-instance bare form. Use it instead of re-deriving the table. +The forms apply identically to `IInvocationExpression.Instance` (built-in record methods and user procedures: `MyProc()`, `this.MyProc()`, `Rec.MyProc()`) and to `IFieldAccess.Instance`. `GetReceiverTableType` in `ALCops.Common/Extensions/OperationExtensions.cs` is the canonical resolver for both, including the null-instance bare form. It takes the member operation itself (the `IInvocationExpression` or `IFieldAccess`, never its `Instance`) because `BoundCall.Instance` is null for any `IsStatic` method as well as for bare self: for an invocation with a null instance it first rejects static built-in targets, then falls back to the containing object. Use it instead of re-deriving the table. The AL 12 netstandard2.1 guard uses `TargetMethod.ContainingSymbol is IClassTypeSymbol` whose name is not `"Table"` (static built-ins live on IsolatedStorage, NumberSequence, Dialog, System, etc.; record built-ins live on the `Table` class). The compiler treats a null receiver and the synthesized self global as the same thing (`OverloadResolution.IsSelf`), and Microsoft's `Rule248AddThis` (CodeCop) recognises bare self with a null `Instance` plus `!symbol.IsSynthesized`, excluding extension objects because `this` there rebinds to the target. A complete "is this the current record" predicate needs four shapes: a null `Instance`, an `IGlobalReferenceExpression` over a synthesized `Rec`, an `ILocalReferenceExpression` over a synthesized `Rec`, and `OperationKind.ThisReference`. diff --git a/.claude/skills/fix-false-positive/references/regression-catalog.md b/.claude/skills/fix-false-positive/references/regression-catalog.md index 8d323f14..951478a7 100644 --- a/.claude/skills/fix-false-positive/references/regression-catalog.md +++ b/.claude/skills/fix-false-positive/references/regression-catalog.md @@ -24,3 +24,4 @@ Recurring causes of false positives/negatives, mined from `fix(...)` commits. Wh | **Table shape: implicit primary key** | A table without a `keys` section has a synthesized primary key over its first valid field. `ITableTypeSymbol.Keys` never lists it (also for referenced `.app` tables); only `PrimaryKey` does. Key-membership logic must read `PrimaryKey`, and every rule reading keys needs a fixture without a `keys` section. | PC0029 #544 | | **One `SymbolKind` covers several AL constructs** | `SymbolKind.Action` spans `area`, `group`, `action`, `separator`, `actionref`, `customaction`, `systemaction` and `fileuploadaction`; `SymbolKind.Control` spans `area`, `group`, `field`, `part` and the rest. Read `IActionSymbol.ActionKind` / `IControlSymbol.ControlKind` before treating a name, caption or property as developer-chosen. | LC0092 #537, AC0011 | | **Version-scoped syntax** (`this`, newer keywords) | Fixtures need `SkipTestIfVersionIsTooLow("14.0")` or `RequireMinimumVersion(...)`; a rule may need a `VersionProvider` gate rather than a code change. | PC0035 fixtures, PC0029 | +| **Null `Instance` means bare self *or* static built-in class** | `BoundCall.Instance` is null for any `IsStatic` target (IsolatedStorage Get/Delete/Contains, NumberSequence Insert/Delete/Next, File Rename/Copy, bare Error/Format), not just bare self. `GetReceiverTableType` takes the invocation or field access itself (never its `Instance`) and returns null for static targets. | PC0013 #550 | diff --git a/src/ALCops.ApplicationCop/Analyzers/UseReturnValueForDatabaseReadMethods.cs b/src/ALCops.ApplicationCop/Analyzers/UseReturnValueForDatabaseReadMethods.cs index 0effe752..ba2e66e8 100644 --- a/src/ALCops.ApplicationCop/Analyzers/UseReturnValueForDatabaseReadMethods.cs +++ b/src/ALCops.ApplicationCop/Analyzers/UseReturnValueForDatabaseReadMethods.cs @@ -31,7 +31,7 @@ private void AnalyzeInvocation(OperationAnalysisContext ctx) if (invocation.TargetMethod.MethodKind != EnumProvider.MethodKind.BuiltInMethod) return; - if (invocation.Instance.GetReceiverTableType(ctx.ContainingSymbol, out _) is null) + if (invocation.GetReceiverTableType(ctx.ContainingSymbol, out _) is null) return; if (!DatabaseReadMethods.Contains(invocation.TargetMethod.Name)) diff --git a/src/ALCops.Common/Extensions/OperationExtensions.cs b/src/ALCops.Common/Extensions/OperationExtensions.cs index 790853c7..da74e719 100644 --- a/src/ALCops.Common/Extensions/OperationExtensions.cs +++ b/src/ALCops.Common/Extensions/OperationExtensions.cs @@ -72,26 +72,61 @@ public static IOperation UnwrapConversions(this IOperation operation) } /// - /// Resolves the table backing the record receiver of an invocation or field access, - /// handling all four AL receiver forms uniformly: + /// Resolves the table backing the record receiver of a member operation: an + /// (built-in record method or user procedure) or an + /// . Handles all four AL receiver forms uniformly: /// - /// MyVar.M() / Rec.M() / this.M(): is non-null; - /// the record type comes from instance.Type. - /// Bare M(): is null; the table comes from + /// MyVar.M() / Rec.M() / this.M(): the operation's Instance is non-null; + /// the record type comes from its Type. + /// Bare M() / F: Instance is null; the table comes from /// 's containing type (table, or target of a table extension). /// + /// The SDK also returns a null Instance for every static built-in call + /// (IsolatedStorage.Get(...), NumberSequence.Next(...), bare Error(...)), so only the + /// target method can tell that apart from a bare self call; a static built-in has no record receiver. /// - /// The IInvocationExpression.Instance (null for bare implicit-self calls). - /// The symbol whose body contains the call (e.g. ctx.ContainingSymbol); - /// used only when is null. + /// The invocation or field access whose receiver is resolved. + /// The symbol whose body contains the operation (e.g. ctx.ContainingSymbol); + /// used only when the receiver is implicit. /// The when available (non-null for variable / Rec / this - /// receivers); null for bare self calls where only the table shape is known. + /// receivers); null for bare self access where only the table shape is known. /// The backing , or null when the receiver is not a record/table - /// (e.g. inside a codeunit or page). + /// (e.g. inside a codeunit or page), the invoked method is static, or is neither + /// an invocation nor a field access. public static ITableTypeSymbol? GetReceiverTableType( - this IOperation? instance, ISymbol? containingSymbol, out IRecordTypeSymbol? recordType) + this IOperation operation, ISymbol? containingSymbol, out IRecordTypeSymbol? recordType) { recordType = null; + IOperation? instance; + + switch (operation) + { + case IInvocationExpression invocation: + if (invocation.Instance is null) + { +#if NETSTANDARD2_1 + // IMethodSymbol.IsStatic does not exist at the AL 12 floor. Static built-ins are declared on a + // language class other than Table (IsolatedStorage, NumberSequence, Dialog, System, ...), while + // the record built-ins live on the Table class and user procedures on the object symbol. + bool isStatic = invocation.TargetMethod is { ContainingSymbol: IClassTypeSymbol containingClass } && + !SemanticFacts.IsSameName(containingClass.Name, "Table"); +#else + bool isStatic = invocation.TargetMethod is { IsStatic: true }; +#endif + if (isStatic) + return null; + } + + instance = invocation.Instance; + break; + + case IFieldAccess fieldAccess: + instance = fieldAccess.Instance; + break; + + default: + return null; + } if (instance is not null) { diff --git a/src/ALCops.Common/Permissions/RequiredPermissionDetector.cs b/src/ALCops.Common/Permissions/RequiredPermissionDetector.cs index d49d586f..923b8224 100644 --- a/src/ALCops.Common/Permissions/RequiredPermissionDetector.cs +++ b/src/ALCops.Common/Permissions/RequiredPermissionDetector.cs @@ -36,7 +36,7 @@ public static class RequiredPermissionDetector if (operation == DatabaseOperation.None) return null; - var tableType = invocation.Instance.GetReceiverTableType(containingSymbol, out var recordType); + var tableType = invocation.GetReceiverTableType(containingSymbol, out var recordType); if (tableType is null || !IsPermissionRelevant(tableType, includeSystemTables)) return null; diff --git a/src/ALCops.DocumentationCop/Analyzers/WriteToFlowFieldRequiresComment.cs b/src/ALCops.DocumentationCop/Analyzers/WriteToFlowFieldRequiresComment.cs index 30036e38..8d7c5984 100644 --- a/src/ALCops.DocumentationCop/Analyzers/WriteToFlowFieldRequiresComment.cs +++ b/src/ALCops.DocumentationCop/Analyzers/WriteToFlowFieldRequiresComment.cs @@ -72,7 +72,7 @@ private void AnalyzeInvocationExpression(OperationAnalysisContext ctx) if (!string.Equals(targetMethod.Name, "Validate", StringComparison.Ordinal)) return; - if (operation.Instance.GetReceiverTableType(ctx.ContainingSymbol, out _) is null) + if (operation.GetReceiverTableType(ctx.ContainingSymbol, out _) is null) return; if (operation.Arguments.Length == 0) diff --git a/src/ALCops.LinterCop/Analyzers/AnalyzeCountMethod.cs b/src/ALCops.LinterCop/Analyzers/AnalyzeCountMethod.cs index 835d91f4..33b0ae74 100644 --- a/src/ALCops.LinterCop/Analyzers/AnalyzeCountMethod.cs +++ b/src/ALCops.LinterCop/Analyzers/AnalyzeCountMethod.cs @@ -47,7 +47,7 @@ private void AnalyzeCountInvocation(OperationAnalysisContext ctx) invocation.TargetMethod.ContainingSymbol?.Name != "Table") return; - var tableType = invocation.Instance.GetReceiverTableType(ctx.ContainingSymbol, out var recordType); + var tableType = invocation.GetReceiverTableType(ctx.ContainingSymbol, out var recordType); if (tableType is null) return; diff --git a/src/ALCops.LinterCop/Analyzers/ExplicitlySetRunTrigger.cs b/src/ALCops.LinterCop/Analyzers/ExplicitlySetRunTrigger.cs index 55be6120..8eab9dfb 100644 --- a/src/ALCops.LinterCop/Analyzers/ExplicitlySetRunTrigger.cs +++ b/src/ALCops.LinterCop/Analyzers/ExplicitlySetRunTrigger.cs @@ -36,7 +36,7 @@ private void AnalyzeRunTriggerParameters(OperationAnalysisContext ctx) if (targetMethod.MethodKind != EnumProvider.MethodKind.BuiltInMethod || !BuiltInMethodNames.Contains(targetMethod.Name)) return; - if (invocation.Instance.GetReceiverTableType(ctx.ContainingSymbol, out _) is null) + if (invocation.GetReceiverTableType(ctx.ContainingSymbol, out _) is null) return; foreach (var arg in invocation.Arguments) diff --git a/src/ALCops.PlatformCop.Test/Rules/RecordGetProcedureArguments/NoDiagnostic/IsolatedStorageGetInTableExtension.al b/src/ALCops.PlatformCop.Test/Rules/RecordGetProcedureArguments/NoDiagnostic/IsolatedStorageGetInTableExtension.al new file mode 100644 index 00000000..9fdcc0f1 --- /dev/null +++ b/src/ALCops.PlatformCop.Test/Rules/RecordGetProcedureArguments/NoDiagnostic/IsolatedStorageGetInTableExtension.al @@ -0,0 +1,28 @@ +table 50100 "My Setup" +{ + fields + { + field(1; "Primary Key"; Integer) { } + } + + keys + { + key(PK; "Primary Key") { Clustered = true; } + } +} + +tableextension 50100 "My Setup Ext" extends "My Setup" +{ + procedure GetPassword(): Text + var + Password: Text; + begin + if IsolatedStorage.Contains('MyKey', DataScope::Module) then + [|IsolatedStorage.Get('MyKey', DataScope::Module, Password)|]; + + [|IsolatedStorage.Get('MyKey', Password)|]; + + IsolatedStorage.Delete('MyKey', DataScope::Module); + exit(Password); + end; +} diff --git a/src/ALCops.PlatformCop.Test/Rules/RecordGetProcedureArguments/NoDiagnostic/IsolatedStorageGetInTableProcedure.al b/src/ALCops.PlatformCop.Test/Rules/RecordGetProcedureArguments/NoDiagnostic/IsolatedStorageGetInTableProcedure.al new file mode 100644 index 00000000..167d0974 --- /dev/null +++ b/src/ALCops.PlatformCop.Test/Rules/RecordGetProcedureArguments/NoDiagnostic/IsolatedStorageGetInTableProcedure.al @@ -0,0 +1,25 @@ +table 50100 "My Setup" +{ + fields + { + field(1; "Primary Key"; Integer) { } + } + + keys + { + key(PK; "Primary Key") { Clustered = true; } + } + + procedure GetPassword(): Text + var + Password: Text; + begin + if IsolatedStorage.Contains('MyKey', DataScope::Module) then + [|IsolatedStorage.Get('MyKey', DataScope::Module, Password)|]; + + [|IsolatedStorage.Get('MyKey', Password)|]; + + IsolatedStorage.Delete('MyKey', DataScope::Module); + exit(Password); + end; +} diff --git a/src/ALCops.PlatformCop.Test/Rules/RecordGetProcedureArguments/NoDiagnostic/IsolatedStorageGetInTableTrigger.al b/src/ALCops.PlatformCop.Test/Rules/RecordGetProcedureArguments/NoDiagnostic/IsolatedStorageGetInTableTrigger.al new file mode 100644 index 00000000..8bcb29a2 --- /dev/null +++ b/src/ALCops.PlatformCop.Test/Rules/RecordGetProcedureArguments/NoDiagnostic/IsolatedStorageGetInTableTrigger.al @@ -0,0 +1,25 @@ +table 50100 "My Setup" +{ + fields + { + field(1; "Primary Key"; Integer) { } + field(2; "Line No."; Integer) { } + } + + keys + { + key(PK; "Primary Key", "Line No.") { Clustered = true; } + } + + trigger OnInsert() + var + Password: Text; + begin + if IsolatedStorage.Contains('MyKey', DataScope::Module) then + [|IsolatedStorage.Get('MyKey', DataScope::Module, Password)|]; + + [|IsolatedStorage.Get('MyKey', Password)|]; + + IsolatedStorage.Delete('MyKey', DataScope::Module); + end; +} diff --git a/src/ALCops.PlatformCop.Test/Rules/RecordGetProcedureArguments/RecordGetProcedureArguments.cs b/src/ALCops.PlatformCop.Test/Rules/RecordGetProcedureArguments/RecordGetProcedureArguments.cs index 637086ed..5cdb215a 100644 --- a/src/ALCops.PlatformCop.Test/Rules/RecordGetProcedureArguments/RecordGetProcedureArguments.cs +++ b/src/ALCops.PlatformCop.Test/Rules/RecordGetProcedureArguments/RecordGetProcedureArguments.cs @@ -77,8 +77,17 @@ public async Task HasDiagnostic(string testCase) [TestCase("RecordGetSetupTableCorrectArgumentsProvided")] [TestCase("RecordGetSetupTableNoArgumentsProvided")] [TestCase("RecordGetXmlPortTableElement")] + [TestCase("IsolatedStorageGetInTableProcedure")] + [TestCase("IsolatedStorageGetInTableTrigger")] + [TestCase("IsolatedStorageGetInTableExtension")] public async Task NoDiagnostic(string testCase) { + SkipTestIfVersionIsTooLow( + ["IsolatedStorageGetInTableExtension"], + testCase, + "13.0", + "AL 12 rejects a tableextension whose target is in the same module (AL0334)."); + var code = await File.ReadAllTextAsync(Path.Combine(_testCasePath, nameof(NoDiagnostic), $"{testCase}.al")) .ConfigureAwait(false); diff --git a/src/ALCops.PlatformCop/Analyzers/PossibleOverflowAssigning.cs b/src/ALCops.PlatformCop/Analyzers/PossibleOverflowAssigning.cs index a28439b9..4f7ec192 100644 --- a/src/ALCops.PlatformCop/Analyzers/PossibleOverflowAssigning.cs +++ b/src/ALCops.PlatformCop/Analyzers/PossibleOverflowAssigning.cs @@ -268,7 +268,7 @@ private void AnalyzeGetInvocation(OperationAnalysisContext ctx, IInvocationExpre if (invocation.Arguments.Length < 1) return; - if (invocation.Instance.GetReceiverTableType(ctx.ContainingSymbol, out _) is not ITableTypeSymbol table) + if (invocation.GetReceiverTableType(ctx.ContainingSymbol, out _) is not ITableTypeSymbol table) return; if (invocation.Arguments.Length < table.PrimaryKey.Fields.Length) diff --git a/src/ALCops.PlatformCop/Analyzers/RecordGetProcedureArguments.cs b/src/ALCops.PlatformCop/Analyzers/RecordGetProcedureArguments.cs index d53190c2..33be6790 100644 --- a/src/ALCops.PlatformCop/Analyzers/RecordGetProcedureArguments.cs +++ b/src/ALCops.PlatformCop/Analyzers/RecordGetProcedureArguments.cs @@ -62,7 +62,7 @@ private void AnalyzeInvocationStatement(OperationAnalysisContext ctx) return; } - if (invocation.Instance.GetReceiverTableType(ctx.ContainingSymbol, out _) is not ITableTypeSymbol table) + if (invocation.GetReceiverTableType(ctx.ContainingSymbol, out _) is not ITableTypeSymbol table) return; if (IsSingletonTable(table)) diff --git a/src/ALCops.PlatformCop/Analyzers/SetRangeWithFilterOperators.cs b/src/ALCops.PlatformCop/Analyzers/SetRangeWithFilterOperators.cs index 02d2ab05..7e627cd8 100644 --- a/src/ALCops.PlatformCop/Analyzers/SetRangeWithFilterOperators.cs +++ b/src/ALCops.PlatformCop/Analyzers/SetRangeWithFilterOperators.cs @@ -32,7 +32,7 @@ private void AnalyzeInvocation(OperationAnalysisContext ctx) if (operation.TargetMethod.MethodKind != EnumProvider.MethodKind.BuiltInMethod || operation.TargetMethod.Name != SetRangeMethodName || - operation.Instance.GetReceiverTableType(ctx.ContainingSymbol, out _) is null || + operation.GetReceiverTableType(ctx.ContainingSymbol, out _) is null || operation.Arguments.Length < 2) return; diff --git a/src/ALCops.PlatformCop/Analyzers/TransferFieldsSchemaCompatibility.cs b/src/ALCops.PlatformCop/Analyzers/TransferFieldsSchemaCompatibility.cs index 1b2168e7..61ca7ff4 100644 --- a/src/ALCops.PlatformCop/Analyzers/TransferFieldsSchemaCompatibility.cs +++ b/src/ALCops.PlatformCop/Analyzers/TransferFieldsSchemaCompatibility.cs @@ -142,7 +142,7 @@ private static void AnalyzeInvocation(OperationAnalysisContext ctx) var sourceTable = TryResolveSymbolFromArgument(invocation) as ITableTypeSymbol; - var targetTable = invocation.Instance.GetReceiverTableType(ctx.ContainingSymbol, out _); + var targetTable = invocation.GetReceiverTableType(ctx.ContainingSymbol, out _); if (sourceTable is null || targetTable is null) return; diff --git a/src/ALCops.PlatformCop/Analyzers/UseSequentialGuid.cs b/src/ALCops.PlatformCop/Analyzers/UseSequentialGuid.cs index c9cf70c6..07109c52 100644 --- a/src/ALCops.PlatformCop/Analyzers/UseSequentialGuid.cs +++ b/src/ALCops.PlatformCop/Analyzers/UseSequentialGuid.cs @@ -405,7 +405,7 @@ private static bool IsValidateCall(IInvocationExpression invocation) => if (fieldSymbol.GetTypeSymbol().GetNavTypeKindSafe() != EnumProvider.NavTypeKind.Guid) return null; - var tableType = fieldAccess.Instance.GetReceiverTableType(containingSymbol, out var recordType); + var tableType = fieldAccess.GetReceiverTableType(containingSymbol, out var recordType); if (tableType is null || tableType.TableType != EnumProvider.TableTypeKind.Normal) return null; @@ -419,7 +419,7 @@ private static bool IsValidateCall(IInvocationExpression invocation) => private static KeyFieldResult? CheckValidateTarget(IInvocationExpression validateCall, ISymbol? containingSymbol = null) { - var tableType = validateCall.Instance.GetReceiverTableType(containingSymbol, out var recordType); + var tableType = validateCall.GetReceiverTableType(containingSymbol, out var recordType); if (tableType is null || tableType.TableType != EnumProvider.TableTypeKind.Normal) return null;