diff --git a/src/ALCops.ApplicationCop.Test/Rules/TableDataAccessRequiresPermissions/HasDiagnostic/DottedTableName.al b/src/ALCops.ApplicationCop.Test/Rules/TableDataAccessRequiresPermissions/HasDiagnostic/DottedTableName.al new file mode 100644 index 00000000..be71813b --- /dev/null +++ b/src/ALCops.ApplicationCop.Test/Rules/TableDataAccessRequiresPermissions/HasDiagnostic/DottedTableName.al @@ -0,0 +1,17 @@ +codeunit 50000 MyCodeunit +{ + procedure MyProcedure() + var + MyTable: Record "ABC Example Header.Line"; + begin + [|MyTable.FindFirst();|] + end; +} + +table 50000 "ABC Example Header.Line" +{ + fields + { + field(1; MyField; Integer) { } + } +} \ No newline at end of file diff --git a/src/ALCops.ApplicationCop.Test/Rules/TableDataAccessRequiresPermissions/HasFix/AddNewPermissionsPropertyDottedName/current.al b/src/ALCops.ApplicationCop.Test/Rules/TableDataAccessRequiresPermissions/HasFix/AddNewPermissionsPropertyDottedName/current.al new file mode 100644 index 00000000..1a1a9e55 --- /dev/null +++ b/src/ALCops.ApplicationCop.Test/Rules/TableDataAccessRequiresPermissions/HasFix/AddNewPermissionsPropertyDottedName/current.al @@ -0,0 +1,18 @@ +codeunit 50000 MyCodeunit +{ + + trigger OnRun() + var + MyTable: Record "ABC Example Header.Line"; + begin + [|MyTable.FindFirst();|] + end; +} + +table 50000 "ABC Example Header.Line" +{ + fields + { + field(1; MyField; Integer) { } + } +} diff --git a/src/ALCops.ApplicationCop.Test/Rules/TableDataAccessRequiresPermissions/HasFix/AddNewPermissionsPropertyDottedName/expected.al b/src/ALCops.ApplicationCop.Test/Rules/TableDataAccessRequiresPermissions/HasFix/AddNewPermissionsPropertyDottedName/expected.al new file mode 100644 index 00000000..0a315fed --- /dev/null +++ b/src/ALCops.ApplicationCop.Test/Rules/TableDataAccessRequiresPermissions/HasFix/AddNewPermissionsPropertyDottedName/expected.al @@ -0,0 +1,18 @@ +codeunit 50000 MyCodeunit +{ + Permissions = tabledata "ABC Example Header.Line" = r; + trigger OnRun() + var + MyTable: Record "ABC Example Header.Line"; + begin + MyTable.FindFirst(); + end; +} + +table 50000 "ABC Example Header.Line" +{ + fields + { + field(1; MyField; Integer) { } + } +} diff --git a/src/ALCops.ApplicationCop.Test/Rules/TableDataAccessRequiresPermissions/HasFix/MergePermissionCharDottedName/current.al b/src/ALCops.ApplicationCop.Test/Rules/TableDataAccessRequiresPermissions/HasFix/MergePermissionCharDottedName/current.al new file mode 100644 index 00000000..ebcf513d --- /dev/null +++ b/src/ALCops.ApplicationCop.Test/Rules/TableDataAccessRequiresPermissions/HasFix/MergePermissionCharDottedName/current.al @@ -0,0 +1,19 @@ +codeunit 50000 MyCodeunit +{ + Permissions = tabledata "ABC Example Header.Line" = r; + + procedure Test() + var + MyTable: Record "ABC Example Header.Line"; + begin + [|MyTable.Modify();|] + end; +} + +table 50000 "ABC Example Header.Line" +{ + fields + { + field(1; MyField; Integer) { } + } +} diff --git a/src/ALCops.ApplicationCop.Test/Rules/TableDataAccessRequiresPermissions/HasFix/MergePermissionCharDottedName/expected.al b/src/ALCops.ApplicationCop.Test/Rules/TableDataAccessRequiresPermissions/HasFix/MergePermissionCharDottedName/expected.al new file mode 100644 index 00000000..2ea50c2d --- /dev/null +++ b/src/ALCops.ApplicationCop.Test/Rules/TableDataAccessRequiresPermissions/HasFix/MergePermissionCharDottedName/expected.al @@ -0,0 +1,19 @@ +codeunit 50000 MyCodeunit +{ + Permissions = tabledata "ABC Example Header.Line" = rm; + + procedure Test() + var + MyTable: Record "ABC Example Header.Line"; + begin + MyTable.Modify(); + end; +} + +table 50000 "ABC Example Header.Line" +{ + fields + { + field(1; MyField; Integer) { } + } +} diff --git a/src/ALCops.ApplicationCop.Test/Rules/TableDataAccessRequiresPermissions/NoDiagnostic/DottedTableNameWithPermissions.al b/src/ALCops.ApplicationCop.Test/Rules/TableDataAccessRequiresPermissions/NoDiagnostic/DottedTableNameWithPermissions.al new file mode 100644 index 00000000..92661d06 --- /dev/null +++ b/src/ALCops.ApplicationCop.Test/Rules/TableDataAccessRequiresPermissions/NoDiagnostic/DottedTableNameWithPermissions.al @@ -0,0 +1,19 @@ +codeunit 50000 MyCodeunit +{ + Permissions = tabledata "ABC Example Header.Line" = r; + + procedure MyProcedure() + var + MyTable: Record "ABC Example Header.Line"; + begin + [|MyTable.FindFirst();|] + end; +} + +table 50000 "ABC Example Header.Line" +{ + fields + { + field(1; MyField; Integer) { } + } +} diff --git a/src/ALCops.ApplicationCop.Test/Rules/TableDataAccessRequiresPermissions/TableDataAccessRequiresPermissions.cs b/src/ALCops.ApplicationCop.Test/Rules/TableDataAccessRequiresPermissions/TableDataAccessRequiresPermissions.cs index 3603f365..ad5f2d6c 100644 --- a/src/ALCops.ApplicationCop.Test/Rules/TableDataAccessRequiresPermissions/TableDataAccessRequiresPermissions.cs +++ b/src/ALCops.ApplicationCop.Test/Rules/TableDataAccessRequiresPermissions/TableDataAccessRequiresPermissions.cs @@ -29,6 +29,7 @@ public void Setup() [TestCase("XmlPorts")] [TestCase("Queries")] [TestCase("Reports")] + [TestCase("DottedTableName")] public async Task HasDiagnostic(string testCase) { var code = await File.ReadAllTextAsync(Path.Combine(_testCasePath, nameof(HasDiagnostic), $"{testCase}.al")) @@ -60,6 +61,7 @@ public async Task HasDiagnostic(string testCase) [TestCase("GetBySystemIdWithPermissions")] [TestCase("CountWithPermissions")] [TestCase("ImplicitSelfCallWithInherentPermissions")] + [TestCase("DottedTableNameWithPermissions")] public async Task NoDiagnostic(string testCase) { SkipTestIfVersionIsTooLow( @@ -84,6 +86,8 @@ public async Task NoDiagnostic(string testCase) [TestCase("AddEntryAlphabetical")] [TestCase("AddEntryAlphabeticalFirst")] [TestCase("AddEntryAppend")] + [TestCase("AddNewPermissionsPropertyDottedName")] + [TestCase("MergePermissionCharDottedName")] public async Task HasFix(string testCase) { var currentCode = await File.ReadAllTextAsync( diff --git a/src/ALCops.ApplicationCop/CodeFixes/TableDataAccessRequiresPermissions.cs b/src/ALCops.ApplicationCop/CodeFixes/TableDataAccessRequiresPermissions.cs index a11f465c..e6b92bbc 100644 --- a/src/ALCops.ApplicationCop/CodeFixes/TableDataAccessRequiresPermissions.cs +++ b/src/ALCops.ApplicationCop/CodeFixes/TableDataAccessRequiresPermissions.cs @@ -125,32 +125,37 @@ private static void RegisterInstanceCodeFix(CodeFixContext ctx, SyntaxNode synta // Resolve table name using C#-like namespace resolution var compilationUnit = objectSyntax.FirstAncestorOrSelf(); - var resolvedTableName = ResolveTableNameForFix(props.TableName, props.TableNamespace, compilationUnit); + var resolved = ResolveTableNameForFix(props.TableName, props.TableNamespace, compilationUnit); ctx.RegisterCodeFix( - CreateCodeAction(objectIdentity, resolvedTableName, props.PermissionChar, document, generateFixAll: true), + CreateCodeAction(objectIdentity, resolved, props.TableName, props.PermissionChar, document, generateFixAll: true), diagnostic); } private static TableDataAccessRequiresPermissionsCodeAction CreateCodeAction( - (SyntaxKind Kind, string? Name) objectIdentity, string tableName, char permissionChar, - Document document, bool generateFixAll) + (SyntaxKind Kind, string? Name) objectIdentity, ResolvedTableName resolved, + string rawTableName, char permissionChar, Document document, bool generateFixAll) { + var displayName = resolved.QualifyingNamespace is not null + ? $"{resolved.QualifyingNamespace}.{resolved.TableName}" + : resolved.TableName; + var title = string.Format( ApplicationCopAnalyzers.TableDataAccessRequiresPermissionsCodeAction, permissionChar, - tableName); + displayName); return new TableDataAccessRequiresPermissionsCodeAction( title, - ct => ApplyFix(document, objectIdentity, tableName, permissionChar, ct), + ct => ApplyFix(document, objectIdentity, resolved, rawTableName, permissionChar, ct), nameof(TableDataAccessRequiresPermissionsCodeFixProvider), generateFixAll); } private static async Task ApplyFix(Document document, (SyntaxKind Kind, string? Name) objectIdentity, - string tableName, char permissionChar, CancellationToken cancellationToken) + ResolvedTableName resolved, string rawTableName, + char permissionChar, CancellationToken cancellationToken) { var root = await document.GetSyntaxRootAsync(cancellationToken) .ConfigureAwait(false); @@ -170,11 +175,11 @@ private static async Task ApplyFix(Document document, if (permissionsProperty is null) { - newPropertyList = AddNewPermissionsProperty(propertyList, tableName, permissionChar); + newPropertyList = AddNewPermissionsProperty(propertyList, resolved, permissionChar); } else { - newPropertyList = UpdateExistingPermissionsProperty(propertyList, permissionsProperty, tableName, permissionChar); + newPropertyList = UpdateExistingPermissionsProperty(propertyList, permissionsProperty, resolved, rawTableName, permissionChar); } var newRoot = root.ReplaceNode(propertyList, newPropertyList); @@ -205,10 +210,10 @@ private static (SyntaxKind Kind, string? Name) GetObjectIdentity(ObjectSyntax ob } private static PropertyListSyntax AddNewPermissionsProperty( - PropertyListSyntax propertyList, string tableName, char permissionChar) + PropertyListSyntax propertyList, ResolvedTableName resolved, char permissionChar) { var permissionEntry = PermissionSyntaxHelper.CreatePermissionSyntax( - tableName, permissionChar.ToString()); + resolved.TableName, resolved.QualifyingNamespace, permissionChar.ToString()); var permissionValue = SyntaxFactory.PermissionPropertyValue() .AddPermissionProperties(permissionEntry); var property = SyntaxFactory.Property( @@ -220,12 +225,18 @@ private static PropertyListSyntax AddNewPermissionsProperty( private static PropertyListSyntax UpdateExistingPermissionsProperty( PropertyListSyntax propertyList, PropertySyntax permissionsProperty, - string tableName, char permissionChar) + ResolvedTableName resolved, string rawTableName, char permissionChar) { if (permissionsProperty.Value is not PermissionPropertyValueSyntax permissionValue) return propertyList; - var existingEntry = PermissionSyntaxHelper.FindExistingEntry(permissionValue, tableName); + // FindExistingEntry compares against GetObjectNameFromPermission output (always unquoted). + // Construct the search name in the same format: raw name for simple, namespace.raw for qualified. + var searchName = resolved.QualifyingNamespace is not null + ? $"{resolved.QualifyingNamespace}.{rawTableName}" + : rawTableName; + + var existingEntry = PermissionSyntaxHelper.FindExistingEntry(permissionValue, searchName); PermissionPropertyValueSyntax newPermissionValue; if (existingEntry is not null) @@ -236,7 +247,7 @@ private static PropertyListSyntax UpdateExistingPermissionsProperty( else { // Table not listed: add a new entry - newPermissionValue = AddNewEntry(permissionValue, tableName, permissionChar); + newPermissionValue = AddNewEntry(permissionValue, resolved, rawTableName, permissionChar); } var newProperty = permissionsProperty.WithValue(newPermissionValue); @@ -257,15 +268,21 @@ private static PermissionPropertyValueSyntax MergePermissionChar( } private static PermissionPropertyValueSyntax AddNewEntry( - PermissionPropertyValueSyntax permissionValue, string tableName, char permissionChar) + PermissionPropertyValueSyntax permissionValue, ResolvedTableName resolved, + string rawTableName, char permissionChar) { var permissions = permissionValue.PermissionProperties; var isSorted = PermissionSyntaxHelper.ArePermissionsSorted(permissions); var isMultiLine = PermissionSyntaxHelper.IsMultiLineFormat(permissionValue); - var insertIndex = PermissionSyntaxHelper.FindInsertionIndex(permissions, tableName, isSorted); + + // FindInsertionIndex compares against GetObjectNameFromPermission output (unquoted). + var sortName = resolved.QualifyingNamespace is not null + ? $"{resolved.QualifyingNamespace}.{rawTableName}" + : rawTableName; + var insertIndex = PermissionSyntaxHelper.FindInsertionIndex(permissions, sortName, isSorted); var newEntry = PermissionSyntaxHelper.CreatePermissionSyntax( - tableName, permissionChar.ToString()); + resolved.TableName, resolved.QualifyingNamespace, permissionChar.ToString()); SeparatedSyntaxList newPermissions; if (isMultiLine) @@ -298,16 +315,16 @@ private static PermissionPropertyValueSyntax AddNewEntry( SemanticFacts.IsSameName(valueText, nameof(PropertyKind.Permissions))); } - private static string ResolveTableNameForFix(string tableName, string tableNamespace, + private static ResolvedTableName ResolveTableNameForFix(string tableName, string tableNamespace, CompilationUnitSyntax? compilationUnit) { if (compilationUnit is null || string.IsNullOrEmpty(tableNamespace)) - return tableName; + return new ResolvedTableName(tableName.QuoteIdentifierIfNeededWithReflection(), null); var fileNamespace = PermissionTableNameResolver.GetFileNamespace(compilationUnit); var importedNamespaces = PermissionTableNameResolver.GetImportedNamespaces(compilationUnit); - return PermissionTableNameResolver.ResolveTableName( + return PermissionTableNameResolver.ResolveTableNameParts( tableName, tableNamespace, fileNamespace, importedNamespaces); } } diff --git a/src/ALCops.Common/Permissions/PermissionSyntaxHelper.cs b/src/ALCops.Common/Permissions/PermissionSyntaxHelper.cs index da65d0b5..516bb552 100644 --- a/src/ALCops.Common/Permissions/PermissionSyntaxHelper.cs +++ b/src/ALCops.Common/Permissions/PermissionSyntaxHelper.cs @@ -114,12 +114,13 @@ private static bool IsUpperCaseConvention(string permissions) /// /// Creates a new PermissionSyntax node for the given table name and permission string. - /// Handles both simple names ("Customer") and qualified names ("MyNamespace.Customer"). + /// When is non-null, creates a qualified name + /// (e.g., MyNamespace."Customer"); otherwise creates a simple identifier. /// - public static PermissionSyntax CreatePermissionSyntax(string tableName, string permissions) + public static PermissionSyntax CreatePermissionSyntax(string tableName, string? qualifyingNamespace, string permissions) { var objectType = SyntaxFactory.Token(EnumProvider.SyntaxKind.TableDataKeyword); - var objectReference = CreateObjectReference(tableName); + var objectReference = CreateObjectReference(tableName, qualifyingNamespace); var permissionsToken = SyntaxFactory.Identifier(permissions); return SyntaxFactory.Permission(objectType, objectReference, permissionsToken); @@ -240,22 +241,32 @@ public static string GetEntryIndentation(PermissionPropertyValueSyntax permissio return null; } - private static ObjectNameOrIdSyntax CreateObjectReference(string tableName) + private static ObjectNameOrIdSyntax CreateObjectReference(string tableName, string? qualifyingNamespace) { - var dotIndex = tableName.LastIndexOf('.'); - if (dotIndex >= 0) + if (qualifyingNamespace is not null) { - var namespacePart = tableName.Substring(0, dotIndex); - var namePart = tableName.Substring(dotIndex + 1); var qualifiedName = SyntaxFactory.QualifiedName( - SyntaxFactory.IdentifierName(namespacePart), - SyntaxFactory.IdentifierName(namePart)); + ParseNamespaceName(qualifyingNamespace), + SyntaxFactory.IdentifierName(tableName)); return SyntaxFactory.ObjectNameOrId(qualifiedName); } return SyntaxFactory.ObjectNameOrId(SyntaxFactory.IdentifierName(tableName)); } + /// + /// Builds a from a namespace string that may contain dots + /// (e.g., "MyPTE.Sales" becomes QualifiedName(IdentifierName("MyPTE"), IdentifierName("Sales"))). + /// + private static NameSyntax ParseNamespaceName(string namespaceName) + { + var segments = namespaceName.Split('.'); + NameSyntax result = SyntaxFactory.IdentifierName(segments[0]); + for (int i = 1; i < segments.Length; i++) + result = SyntaxFactory.QualifiedName(result, SyntaxFactory.IdentifierName(segments[i])); + return result; + } + /// /// Inserts a permission entry into a multi-line permission list, preserving /// the multi-line format by fixing separator trivia after insertion. diff --git a/src/ALCops.Common/Permissions/PermissionTableNameResolver.cs b/src/ALCops.Common/Permissions/PermissionTableNameResolver.cs index faf245ac..59ad9c68 100644 --- a/src/ALCops.Common/Permissions/PermissionTableNameResolver.cs +++ b/src/ALCops.Common/Permissions/PermissionTableNameResolver.cs @@ -4,6 +4,22 @@ namespace ALCops.Common.Permissions; +#if NETSTANDARD2_1 +public readonly struct ResolvedTableName +{ + public string TableName { get; } + public string? QualifyingNamespace { get; } + + public ResolvedTableName(string tableName, string? qualifyingNamespace) + { + TableName = tableName; + QualifyingNamespace = qualifyingNamespace; + } +} +#else +public readonly record struct ResolvedTableName(string TableName, string? QualifyingNamespace); +#endif + /// /// Resolves the appropriate table name for use in a Permissions property, /// using C#-like namespace resolution: simple name when the table's namespace @@ -30,6 +46,26 @@ public static string ResolveTableName(string tableName, string? tableNamespace, return $"{tableNamespace}.{tableName.QuoteIdentifierIfNeededWithReflection()}"; } + /// + /// Returns the table name and optional qualifying namespace as separate values, + /// avoiding the need to re-parse a combined string. + /// + public static ResolvedTableName ResolveTableNameParts(string tableName, string? tableNamespace, string? containingNamespace, IEnumerable importedNamespaces) + { + var quotedName = tableName.QuoteIdentifierIfNeededWithReflection(); + + if (string.IsNullOrEmpty(tableNamespace)) + return new ResolvedTableName(quotedName, null); + + if (containingNamespace is not null && SemanticFacts.IsSameName(tableNamespace, containingNamespace)) + return new ResolvedTableName(quotedName, null); + + if (importedNamespaces.Any(ns => SemanticFacts.IsSameName(ns, tableNamespace))) + return new ResolvedTableName(quotedName, null); + + return new ResolvedTableName(quotedName, tableNamespace); + } + /// /// Extracts the namespace from a CompilationUnitSyntax. ///