diff --git a/apps/website/.eslintrc.custom.json b/apps/website/.eslintrc.custom.json index 71f6941e4..66dff9094 100644 --- a/apps/website/.eslintrc.custom.json +++ b/apps/website/.eslintrc.custom.json @@ -48,7 +48,8 @@ "rules": { "display-no-domain-models": "error", "display-no-business-logic": "error", - "model-no-domain-in-display": "error" + "model-no-domain-in-display": "error", + "filename-display-match": "error" } }, { diff --git a/apps/website/.eslintrc.json b/apps/website/.eslintrc.json index ea8181fec..db6c26f94 100644 --- a/apps/website/.eslintrc.json +++ b/apps/website/.eslintrc.json @@ -27,6 +27,17 @@ "no-restricted-syntax": "off" } }, + { + "files": [ + "lib/presenters/**/*.ts", + "lib/presenters/**/*.tsx", + "lib/view-models/**/*.ts", + "lib/view-models/**/*.tsx" + ], + "rules": { + "gridpilot-rules/presenter-contract": "error" + } + }, { "files": [ "lib/builders/view-models/*.ts", @@ -42,7 +53,18 @@ "lib/builders/view-data/*.tsx" ], "rules": { - "gridpilot-rules/view-data-builder-contract": "error" + "gridpilot-rules/view-data-builder-contract": "error", + "gridpilot-rules/single-export-per-file": "error", + "gridpilot-rules/filename-matches-export": "error" + } + }, + { + "files": [ + "lib/builders/**/*.ts", + "lib/builders/**/*.tsx" + ], + "rules": { + "gridpilot-rules/no-page-dtos-directory": "error" } }, { @@ -105,7 +127,8 @@ "rules": { "gridpilot-rules/display-no-domain-models": "error", "gridpilot-rules/display-no-business-logic": "error", - "gridpilot-rules/model-no-domain-in-display": "error" + "gridpilot-rules/model-no-domain-in-display": "error", + "gridpilot-rules/filename-display-match": "error" } }, { @@ -117,7 +140,22 @@ "gridpilot-rules/page-query-filename": "error", "gridpilot-rules/page-query-contract": "error", "gridpilot-rules/page-query-execute": "error", - "gridpilot-rules/page-query-return-type": "error" + "gridpilot-rules/page-query-return-type": "error", + "gridpilot-rules/page-query-must-use-builders": "error", + "gridpilot-rules/single-export-per-file": "error", + "gridpilot-rules/filename-matches-export": "error", + "gridpilot-rules/clean-error-handling": "error" + } + }, + { + "files": [ + "lib/mutations/**/*.ts" + ], + "rules": { + "gridpilot-rules/mutation-contract": "error", + "gridpilot-rules/clean-error-handling": "error", + "gridpilot-rules/single-export-per-file": "error", + "gridpilot-rules/filename-matches-export": "error" } }, { @@ -136,7 +174,9 @@ "rules": { "gridpilot-rules/services-no-external-api": "error", "gridpilot-rules/services-must-be-pure": "error", - "gridpilot-rules/filename-service-match": "error" + "gridpilot-rules/filename-service-match": "error", + "gridpilot-rules/services-must-return-result": "error", + "gridpilot-rules/services-implement-contract": "error" } }, { @@ -167,6 +207,27 @@ "rules": { "gridpilot-rules/model-no-display-in-domain": "error" } + }, + { + "files": [ + "lib/**/*.ts", + "lib/**/*.tsx" + ], + "rules": { + "gridpilot-rules/lib-no-next-imports": "error" + } + }, + { + "files": [ + "lib/services/**/*.ts" + ], + "rules": { + "gridpilot-rules/service-function-format": "error", + "gridpilot-rules/services-must-be-marked": "error", + "gridpilot-rules/services-must-be-pure": "error", + "gridpilot-rules/services-no-external-api": "error", + "gridpilot-rules/services-no-instantiation": "error" + } } ], "plugins": [ diff --git a/apps/website/eslint-rules/clean-error-handling.js b/apps/website/eslint-rules/clean-error-handling.js new file mode 100644 index 000000000..a199417fc --- /dev/null +++ b/apps/website/eslint-rules/clean-error-handling.js @@ -0,0 +1,118 @@ +/** + * ESLint rule to enforce clean error handling architecture + * + * PageQueries and Mutations must: + * 1. Use Services for data access + * 2. Services must return Result types + */ + +module.exports = { + meta: { + type: 'problem', + docs: { + description: 'Enforce clean error handling architecture in PageQueries and Mutations', + category: 'Architecture', + recommended: true, + }, + fixable: null, + schema: [], + messages: { + mustUseServices: 'PageQueries and Mutations must use Services for data access, not API Clients directly.', + servicesMustReturnResult: 'Services must return Result for type-safe error handling.', + }, + }, + + create(context) { + const filename = context.getFilename(); + const isPageQuery = filename.includes('/lib/page-queries/'); + const isMutation = filename.includes('/lib/mutations/'); + const isService = filename.includes('/lib/services/'); + const isRelevant = isPageQuery || isMutation || isService; + + if (!isRelevant) return {}; + + // Track imports + const apiClientImports = new Set(); + const serviceImports = new Set(); + + return { + // Track imports + ImportDeclaration(node) { + node.specifiers.forEach(spec => { + const importPath = node.source.value; + if (importPath.includes('/lib/api/')) { + apiClientImports.add(spec.local.name); + } + if (importPath.includes('/lib/services/')) { + serviceImports.add(spec.local.name); + } + }); + }, + + // Check PageQueries/Mutations for direct API Client usage + NewExpression(node) { + if (node.callee.type === 'Identifier') { + const className = node.callee.name; + + // Only check in PageQueries and Mutations + if ((isPageQuery || isMutation) && + (className.endsWith('ApiClient') || className.endsWith('Api'))) { + context.report({ + node, + messageId: 'mustUseServices', + }); + } + } + }, + + // Check Services for Result return type + MethodDefinition(node) { + if (isService && node.key.type === 'Identifier' && node.key.name === 'execute') { + const returnType = node.value.returnType; + + if (!returnType || + !returnType.typeAnnotation || + !returnType.typeAnnotation.typeName || + returnType.typeAnnotation.typeName.name !== 'Promise') { + // Missing Promise return type + context.report({ + node, + messageId: 'servicesMustReturnResult', + }); + return; + } + + // Check for Result type + const typeArgs = returnType.typeAnnotation.typeParameters; + if (!typeArgs || !typeArgs.params || typeArgs.params.length === 0) { + context.report({ + node, + messageId: 'servicesMustReturnResult', + }); + return; + } + + const resultType = typeArgs.params[0]; + if (resultType.type !== 'TSTypeReference' || + !resultType.typeName || + (resultType.typeName.type === 'Identifier' && resultType.typeName.name !== 'Result')) { + context.report({ + node, + messageId: 'servicesMustReturnResult', + }); + } + } + }, + + // Check that PageQueries/Mutations have Service imports + 'Program:exit'() { + if ((isPageQuery || isMutation) && serviceImports.size === 0) { + context.report({ + node: context.getSourceCode().ast, + messageId: 'mustUseServices', + }); + } + }, + }; + }, +}; diff --git a/apps/website/eslint-rules/index.js b/apps/website/eslint-rules/index.js index 2e7d516ea..3f9a27734 100644 --- a/apps/website/eslint-rules/index.js +++ b/apps/website/eslint-rules/index.js @@ -37,6 +37,9 @@ const pageQueryMustUseBuilders = require('./page-query-must-use-builders'); const serviceFunctionFormat = require('./service-function-format'); const libNoNextImports = require('./lib-no-next-imports'); const servicesNoInstantiation = require('./services-no-instantiation'); +const noPageDtosDirectory = require('./no-page-dtos-directory'); +const cleanErrorHandling = require('./clean-error-handling'); +const servicesImplementContract = require('./services-implement-contract'); module.exports = { rules: { @@ -82,6 +85,7 @@ module.exports = { 'services-must-be-marked': servicesRules['services-must-be-marked'], 'services-no-external-api': servicesRules['no-external-api-in-services'], 'services-must-be-pure': servicesRules['services-must-be-pure'], + 'services-must-return-result': cleanErrorHandling, // Client-Only Rules 'client-only-no-server-code': clientOnlyRules['no-server-code-in-client-only'], @@ -127,6 +131,13 @@ module.exports = { 'service-function-format': serviceFunctionFormat, 'lib-no-next-imports': libNoNextImports, 'services-no-instantiation': servicesNoInstantiation, + + // Page DTO Rules + 'no-page-dtos-directory': noPageDtosDirectory, + + // Clean Error Handling Rules + 'clean-error-handling': cleanErrorHandling, + 'services-implement-contract': servicesImplementContract, }, // Configurations for different use cases diff --git a/apps/website/eslint-rules/no-page-dtos-directory.js b/apps/website/eslint-rules/no-page-dtos-directory.js new file mode 100644 index 000000000..855760cdf --- /dev/null +++ b/apps/website/eslint-rules/no-page-dtos-directory.js @@ -0,0 +1,37 @@ +/** + * ESLint rule to forbid lib/builders/page-dtos directory + * + * This directory is completely forbidden. + */ + +module.exports = { + meta: { + type: 'problem', + docs: { + description: 'Forbid lib/builders/page-dtos directory', + category: 'Best Practices', + recommended: true, + }, + fixable: null, + schema: [], + messages: { + forbiddenDirectory: 'The lib/builders/page-dtos directory is forbidden. Use lib/builders/view-data/ or lib/builders/view-models/ instead.', + }, + }, + + create(context) { + const filename = context.getFilename(); + + return { + Program(node) { + // Check if file is in the forbidden directory + if (filename.includes('lib/builders/page-dtos')) { + context.report({ + node, + messageId: 'forbiddenDirectory', + }); + } + }, + }; + }, +}; diff --git a/apps/website/eslint-rules/page-query-must-use-builders.js b/apps/website/eslint-rules/page-query-must-use-builders.js index 503f1baaf..7f569f275 100644 --- a/apps/website/eslint-rules/page-query-must-use-builders.js +++ b/apps/website/eslint-rules/page-query-must-use-builders.js @@ -1,7 +1,7 @@ /** * ESLint rule to enforce PageQueries use Builders * - * PageQueries should not manually transform API DTOs. + * PageQueries should not manually transform API DTOs or return them directly. * They must use Builder classes to transform API DTOs to View Data. */ @@ -16,7 +16,7 @@ module.exports = { fixable: null, schema: [], messages: { - mustUseBuilder: 'PageQueries must use Builder classes to transform API DTOs. Found manual object literal transformation in execute() method.', + mustUseBuilder: 'PageQueries must use Builder classes to transform API DTOs to View Data. Found manual transformation or direct API DTO return.', multipleExports: 'PageQuery files should only export the PageQuery class, not DTOs.', }, }, @@ -25,8 +25,17 @@ module.exports = { let inPageQueryExecute = false; let hasManualTransformation = false; let hasMultipleExports = false; + let hasBuilderCall = false; + let pageQueryClassName = null; return { + // Track PageQuery class name + ClassDeclaration(node) { + if (node.id && node.id.name && node.id.name.endsWith('PageQuery')) { + pageQueryClassName = node.id.name; + } + }, + // Track PageQuery class execute method MethodDefinition(node) { if (node.key.type === 'Identifier' && @@ -41,19 +50,75 @@ module.exports = { } }, + // Detect Builder calls + CallExpression(node) { + if (inPageQueryExecute) { + // Check for Builder.build() or Builder.createViewData() + if (node.callee.type === 'MemberExpression' && + node.callee.property.type === 'Identifier' && + (node.callee.property.name === 'build' || node.callee.property.name === 'createViewData')) { + + // Check if the object is a Builder + if (node.callee.object.type === 'Identifier' && + (node.callee.object.name.includes('Builder') || + node.callee.object.name.includes('builder'))) { + hasBuilderCall = true; + } + } + } + }, + // Detect object literal assignments (manual transformation) VariableDeclarator(node) { if (inPageQueryExecute && node.init && node.init.type === 'ObjectExpression') { - // This is: const dto = { ... } hasManualTransformation = true; } }, // Detect object literal in return statements ReturnStatement(node) { - if (inPageQueryExecute && node.argument && node.argument.type === 'ObjectExpression') { - // This is: return { ... } - hasManualTransformation = true; + if (inPageQueryExecute && node.argument) { + // Direct object literal return + if (node.argument.type === 'ObjectExpression') { + hasManualTransformation = true; + } + // Direct identifier return (likely API DTO) + else if (node.argument.type === 'Identifier' && + !node.argument.name.includes('ViewData') && + !node.argument.name.includes('viewData')) { + // This might be returning an API DTO directly + // We'll flag it as manual transformation since no builder was used + if (!hasBuilderCall) { + hasManualTransformation = true; + } + } + // CallExpression like Result.ok(apiDto) or Result.err() + else if (node.argument.type === 'CallExpression') { + // Check if it's a Result call with an identifier argument + const callExpr = node.argument; + if (callExpr.callee.type === 'MemberExpression' && + callExpr.callee.object.type === 'Identifier' && + callExpr.callee.object.name === 'Result' && + callExpr.callee.property.type === 'Identifier' && + (callExpr.callee.property.name === 'ok' || callExpr.callee.property.name === 'err')) { + + // If it's Result.ok(someIdentifier), check if the identifier is likely an API DTO + if (callExpr.callee.property.name === 'ok' && + callExpr.arguments.length > 0 && + callExpr.arguments[0].type === 'Identifier') { + const argName = callExpr.arguments[0].name; + // Common API DTO naming patterns + const isApiDto = argName.includes('Dto') || + argName.includes('api') || + argName === 'result' || + argName === 'data'; + + if (isApiDto && !hasBuilderCall) { + hasManualTransformation = true; + } + } + } + } } }, @@ -81,7 +146,8 @@ module.exports = { }, 'Program:exit'() { - if (hasManualTransformation) { + // Only report if no builder was used + if (hasManualTransformation && !hasBuilderCall) { context.report({ node: context.getSourceCode().ast, messageId: 'mustUseBuilder', diff --git a/apps/website/eslint-rules/service-function-format.js b/apps/website/eslint-rules/service-function-format.js index 6bd63f3f1..8a77b72a0 100644 --- a/apps/website/eslint-rules/service-function-format.js +++ b/apps/website/eslint-rules/service-function-format.js @@ -1,10 +1,12 @@ /** * ESLint rule to enforce Service function format - * + * * Services in lib/services/ must: * 1. Be classes named *Service (not functions) - * 2. Not have side effects (no redirect, console.log, etc.) - * 3. Use builders for data transformation + * 2. Create their own dependencies (API Client, Logger, ErrorReporter) + * 3. Return Result types + * 4. NOT use redirect() or process.exit() + * 5. CAN use console.error() for logging (allowed) */ module.exports = { @@ -19,20 +21,22 @@ module.exports = { schema: [], messages: { notAClass: 'Services must be classes named *Service, not functions. Found function "{{name}}" in lib/services/', - hasSideEffects: 'Services must be pure. Found side effect: {{effect}}', noRedirect: 'Services cannot use redirect(). Use PageQueries or Client Components for navigation.', + noProcessExit: 'Services cannot use process.exit().', multipleExports: 'Service files should only export the Service class.', + mustReturnResult: 'Service methods must return Result.', }, }, create(context) { const filename = context.getFilename(); const isInServices = filename.includes('/lib/services/'); - let hasSideEffect = false; - let sideEffectType = ''; + let hasRedirect = false; + let hasProcessExit = false; let hasMultipleExports = false; let hasFunctionExport = false; let functionName = ''; + let hasResultReturningMethod = false; return { // Track function declarations @@ -59,30 +63,20 @@ module.exports = { } }, - // Track redirect calls + // Track redirect and process.exit calls CallExpression(node) { if (isInServices) { // Check for redirect() if (node.callee.type === 'Identifier' && node.callee.name === 'redirect') { - hasSideEffect = true; - sideEffectType = 'redirect()'; + hasRedirect = true; } - // Check for console.log, console.error, etc. - if (node.callee.type === 'MemberExpression' && - node.callee.object.type === 'Identifier' && - node.callee.object.name === 'console') { - hasSideEffect = true; - sideEffectType = 'console.' + (node.callee.property.name || 'call'); - } - // Check for process.exit() if (node.callee.type === 'MemberExpression' && node.callee.object.type === 'Identifier' && node.callee.object.name === 'process' && node.callee.property.name === 'exit') { - hasSideEffect = true; - sideEffectType = 'process.exit()'; + hasProcessExit = true; } } }, @@ -109,6 +103,30 @@ module.exports = { } }, + // Check for Result-returning methods + MethodDefinition(node) { + if (isInServices && node.key.type === 'Identifier') { + const returnType = node.value?.returnType; + + if (returnType && + returnType.typeAnnotation && + returnType.typeAnnotation.typeName && + returnType.typeAnnotation.typeName.name === 'Promise' && + returnType.typeAnnotation.typeParameters && + returnType.typeAnnotation.typeParameters.params.length > 0) { + + const resultType = returnType.typeAnnotation.typeParameters.params[0]; + + if (resultType.type === 'TSTypeReference' && + resultType.typeName && + resultType.typeName.type === 'Identifier' && + resultType.typeName.name === 'Result') { + hasResultReturningMethod = true; + } + } + } + }, + 'Program:exit'() { if (!isInServices) return; @@ -121,12 +139,19 @@ module.exports = { }); } - // Check for side effects - if (hasSideEffect) { + // Check for redirect + if (hasRedirect) { context.report({ node: context.getSourceCode().ast, - messageId: sideEffectType === 'redirect()' ? 'noRedirect' : 'hasSideEffects', - data: { effect: sideEffectType }, + messageId: 'noRedirect', + }); + } + + // Check for process.exit + if (hasProcessExit) { + context.report({ + node: context.getSourceCode().ast, + messageId: 'noProcessExit', }); } @@ -137,6 +162,10 @@ module.exports = { messageId: 'multipleExports', }); } + + // Check for Result-returning methods (warn if none found) + // Note: This is a soft check - services might have private methods that don't return Result + // The important thing is that the public API methods do }, }; }, diff --git a/apps/website/eslint-rules/services-implement-contract.js b/apps/website/eslint-rules/services-implement-contract.js new file mode 100644 index 000000000..f2f29db3c --- /dev/null +++ b/apps/website/eslint-rules/services-implement-contract.js @@ -0,0 +1,133 @@ +/** + * ESLint rule to enforce Services follow clean architecture patterns + * + * Services must: + * 1. Return Result types for type-safe error handling + * 2. Use DomainError types (not strings) + * 3. Be classes named *Service + * 4. Create their own dependencies (API Client, Logger, ErrorReporter) + * 5. Have NO constructor parameters (self-contained) + * + * Note: Method names can vary (execute(), getSomething(), etc.) + */ + +module.exports = { + meta: { + type: 'problem', + docs: { + description: 'Enforce Services follow clean architecture patterns', + category: 'Services', + recommended: true, + }, + fixable: null, + schema: [], + messages: { + mustReturnResult: 'Service methods must return Promise>', + mustUseDomainError: 'Error types must be DomainError objects, not strings', + noConstructorParams: 'Services must be self-contained. Constructor cannot have parameters. Dependencies should be created inside the constructor.', + }, + }, + + create(context) { + const filename = context.getFilename(); + const isInServices = filename.includes('/lib/services/'); + + if (!isInServices) return {}; + + let hasResultReturningMethod = false; + let usesStringErrors = false; + let hasConstructorParams = false; + let classNode = null; + + return { + ClassDeclaration(node) { + classNode = node; + const className = node.id?.name; + + if (!className || !className.endsWith('Service')) { + return; // Not a service class + } + + // Check all methods for Result return types + node.body.body.forEach(member => { + if (member.type === 'MethodDefinition' && + member.key.type === 'Identifier') { + + // Check constructor parameters + if (member.kind === 'constructor') { + const params = member.value.params; + if (params && params.length > 0) { + hasConstructorParams = true; + } + } + + // Check return types + const returnType = member.value?.returnType; + + if (returnType && + returnType.typeAnnotation && + returnType.typeAnnotation.typeName && + returnType.typeAnnotation.typeName.name === 'Promise' && + returnType.typeAnnotation.typeParameters && + returnType.typeAnnotation.typeParameters.params.length > 0) { + + const resultType = returnType.typeAnnotation.typeParameters.params[0]; + + // Check for Result<...> + if (resultType.type === 'TSTypeReference' && + resultType.typeName && + resultType.typeName.type === 'Identifier' && + resultType.typeName.name === 'Result') { + hasResultReturningMethod = true; + } + + // Check for string error type + if (resultType.type === 'TSTypeReference' && + resultType.typeParameters && + resultType.typeParameters.params.length > 1) { + + const errorType = resultType.typeParameters.params[1]; + // Check if error is string literal or string type + if (errorType.type === 'TSStringKeyword' || + (errorType.type === 'TSLiteralType' && errorType.literal.type === 'StringLiteral')) { + usesStringErrors = true; + } + } + } + } + }); + }, + + 'Program:exit'() { + if (!isInServices || !classNode) return; + + const className = classNode.id?.name; + if (!className || !className.endsWith('Service')) return; + + // Error if constructor has parameters + if (hasConstructorParams) { + context.report({ + node: classNode, + messageId: 'noConstructorParams', + }); + } + + // Error if no methods return Result + if (!hasResultReturningMethod) { + context.report({ + node: classNode, + messageId: 'mustReturnResult', + }); + } + + // Error if using string errors + if (usesStringErrors) { + context.report({ + node: classNode, + messageId: 'mustUseDomainError', + }); + } + }, + }; + }, +}; diff --git a/apps/website/eslint-rules/test-rule-debug.js b/apps/website/eslint-rules/test-rule-debug.js new file mode 100644 index 000000000..37906f026 --- /dev/null +++ b/apps/website/eslint-rules/test-rule-debug.js @@ -0,0 +1,62 @@ +/** + * Debug test for page-query-must-use-builders rule + */ + +const rule = require('./page-query-must-use-builders.js'); + +// Simulate the AST traversal with logging +const mockContext = { + getSourceCode: () => ({ ast: {} }), + report: (data) => { + console.log('✓ REPORT TRIGGERED:', data.messageId); + } +}; + +const visitor = rule.create(mockContext); + +console.log('=== Starting Debug Test ===\n'); + +// 1. ClassDeclaration +console.log('1. ClassDeclaration: AdminDashboardPageQuery'); +visitor.ClassDeclaration({ + id: { name: 'AdminDashboardPageQuery' } +}); + +// 2. MethodDefinition (execute) +console.log('2. MethodDefinition: execute()'); +visitor.MethodDefinition({ + key: { type: 'Identifier', name: 'execute' }, + parent: { type: 'ClassBody' } +}); + +// 3. VariableDeclarator +console.log('3. VariableDeclarator: const apiDto = ...'); +visitor.VariableDeclarator({ + init: { type: 'CallExpression' } +}); + +// 4. ReturnStatement +console.log('4. ReturnStatement: return Result.ok(apiDto)'); +visitor.ReturnStatement({ + argument: { + type: 'CallExpression', + callee: { + type: 'MemberExpression', + object: { type: 'Identifier', name: 'Result' }, + property: { type: 'Identifier', name: 'ok' } + }, + arguments: [{ type: 'Identifier', name: 'apiDto' }] + } +}); + +// 5. MethodDefinition:exit +console.log('5. MethodDefinition:exit'); +visitor['MethodDefinition:exit']({ + key: { type: 'Identifier', name: 'execute' } +}); + +// 6. Program:exit +console.log('6. Program:exit'); +visitor['Program:exit'](); + +console.log('\n=== Test Complete ==='); diff --git a/apps/website/eslint-rules/test-rule-fixed.js b/apps/website/eslint-rules/test-rule-fixed.js new file mode 100644 index 000000000..a5ea2a886 --- /dev/null +++ b/apps/website/eslint-rules/test-rule-fixed.js @@ -0,0 +1,58 @@ +/** + * Fixed test with proper parent structure + */ + +const rule = require('./page-query-must-use-builders.js'); + +const mockContext = { + getSourceCode: () => ({ ast: {} }), + report: (data) => { + console.log('✓ REPORT:', data.messageId); + } +}; + +const visitor = rule.create(mockContext); + +console.log('=== Fixed Test ===\n'); + +// 1. ClassDeclaration +const classNode = { + id: { name: 'AdminDashboardPageQuery' } +}; +visitor.ClassDeclaration(classNode); + +// 2. MethodDefinition (with proper parent structure) +const methodNode = { + key: { type: 'Identifier', name: 'execute' }, + parent: { + type: 'ClassBody', + parent: classNode // This is what was missing! + } +}; +visitor.MethodDefinition(methodNode); + +// 3. VariableDeclarator +visitor.VariableDeclarator({ + init: { type: 'CallExpression' } +}); + +// 4. ReturnStatement +visitor.ReturnStatement({ + argument: { + type: 'CallExpression', + callee: { + type: 'MemberExpression', + object: { type: 'Identifier', name: 'Result' }, + property: { type: 'Identifier', name: 'ok' } + }, + arguments: [{ type: 'Identifier', name: 'apiDto' }] + } +}); + +// 5. MethodDefinition:exit +visitor['MethodDefinition:exit'](methodNode); + +// 6. Program:exit +visitor['Program:exit'](); + +console.log('\n=== Done ==='); diff --git a/apps/website/eslint-rules/test-rule-simple.js b/apps/website/eslint-rules/test-rule-simple.js new file mode 100644 index 000000000..4f988e1f7 --- /dev/null +++ b/apps/website/eslint-rules/test-rule-simple.js @@ -0,0 +1,58 @@ +/** + * Simple test for page-query-must-use-builders rule + */ + +const rule = require('./page-query-must-use-builders.js'); + +// Simulate the AST traversal +const mockContext = { + getSourceCode: () => ({ ast: {} }), + report: (data) => { + console.log('REPORT:', data.messageId); + } +}; + +const visitor = rule.create(mockContext); + +// Simulate visiting AdminDashboardPageQuery +console.log('Starting test...'); + +// 1. ClassDeclaration +visitor.ClassDeclaration({ + id: { name: 'AdminDashboardPageQuery' } +}); + +// 2. MethodDefinition (execute) +visitor.MethodDefinition({ + key: { type: 'Identifier', name: 'execute' }, + parent: { type: 'ClassBody' } +}); + +// 3. VariableDeclarator (const apiDto = ...) - NOT an ObjectExpression +visitor.VariableDeclarator({ + init: { type: 'CallExpression' } // apiDto = await adminService.getDashboardStats() +}); + +// 4. ReturnStatement (return Result.ok(apiDto)) +// This is a CallExpression with callee = Result.ok and argument = apiDto +visitor.ReturnStatement({ + argument: { + type: 'CallExpression', + callee: { + type: 'MemberExpression', + object: { type: 'Identifier', name: 'Result' }, + property: { type: 'Identifier', name: 'ok' } + }, + arguments: [{ type: 'Identifier', name: 'apiDto' }] + } +}); + +// 5. MethodDefinition:exit +visitor['MethodDefinition:exit']({ + key: { type: 'Identifier', name: 'execute' } +}); + +// 6. Program:exit +visitor['Program:exit'](); + +console.log('Test complete'); diff --git a/apps/website/eslint-rules/test-rule-with-logging.js b/apps/website/eslint-rules/test-rule-with-logging.js new file mode 100644 index 000000000..8b42c3943 --- /dev/null +++ b/apps/website/eslint-rules/test-rule-with-logging.js @@ -0,0 +1,190 @@ +/** + * Test with logging inside the rule + */ + +const rule = { + meta: { + type: 'problem', + docs: { + description: 'Test', + category: 'Page Query', + }, + fixable: null, + schema: [], + messages: { + mustUseBuilder: 'Must use builders', + }, + }, + + create(context) { + let inPageQueryExecute = false; + let hasManualTransformation = false; + let hasBuilderCall = false; + let pageQueryClassName = null; + + return { + ClassDeclaration(node) { + if (node.id && node.id.name && node.id.name.endsWith('PageQuery')) { + pageQueryClassName = node.id.name; + console.log(' [ClassDeclaration] Found:', pageQueryClassName); + } + }, + + MethodDefinition(node) { + if (node.key.type === 'Identifier' && + node.key.name === 'execute' && + node.parent.type === 'ClassBody') { + + const classNode = node.parent.parent; + if (classNode && classNode.id && classNode.id.name && + classNode.id.name.endsWith('PageQuery')) { + inPageQueryExecute = true; + console.log(' [MethodDefinition] execute() in PageQuery'); + } + } + }, + + CallExpression(node) { + if (inPageQueryExecute) { + console.log(' [CallExpression] inside execute:', node.callee.type); + if (node.callee.type === 'MemberExpression' && + node.callee.property.type === 'Identifier' && + (node.callee.property.name === 'build' || node.callee.property.name === 'createViewData')) { + console.log(' [CallExpression] Found Builder call!'); + hasBuilderCall = true; + } + } + }, + + VariableDeclarator(node) { + if (inPageQueryExecute && node.init && node.init.type === 'ObjectExpression') { + console.log(' [VariableDeclarator] ObjectExpression found'); + hasManualTransformation = true; + } + }, + + ReturnStatement(node) { + if (inPageQueryExecute && node.argument) { + console.log(' [ReturnStatement] type:', node.argument.type); + + // Direct object literal return + if (node.argument.type === 'ObjectExpression') { + console.log(' [ReturnStatement] ObjectExpression - setting manual transformation'); + hasManualTransformation = true; + } + // Direct identifier return + else if (node.argument.type === 'Identifier') { + console.log(' [ReturnStatement] Identifier:', node.argument.name); + if (!node.argument.name.includes('ViewData') && !node.argument.name.includes('viewData')) { + console.log(' [ReturnStatement] Not ViewData, checking builder call...'); + if (!hasBuilderCall) { + console.log(' [ReturnStatement] No builder call - setting manual transformation'); + hasManualTransformation = true; + } + } + } + // CallExpression + else if (node.argument.type === 'CallExpression') { + console.log(' [ReturnStatement] CallExpression'); + const callExpr = node.argument; + if (callExpr.callee.type === 'MemberExpression' && + callExpr.callee.object.type === 'Identifier' && + callExpr.callee.object.name === 'Result' && + callExpr.callee.property.type === 'Identifier' && + (callExpr.callee.property.name === 'ok' || callExpr.callee.property.name === 'err')) { + + console.log(' [ReturnStatement] Result.' + callExpr.callee.property.name + '()'); + + if (callExpr.callee.property.name === 'ok' && + callExpr.arguments.length > 0 && + callExpr.arguments[0].type === 'Identifier') { + const argName = callExpr.arguments[0].name; + console.log(' [ReturnStatement] Argument:', argName); + + const isApiDto = argName.includes('Dto') || + argName.includes('api') || + argName === 'result' || + argName === 'data'; + + console.log(' [ReturnStatement] Is API DTO?', isApiDto); + console.log(' [ReturnStatement] Has builder call?', hasBuilderCall); + + if (isApiDto && !hasBuilderCall) { + console.log(' [ReturnStatement] ✓ Setting manual transformation!'); + hasManualTransformation = true; + } + } + } + } + } + }, + + 'MethodDefinition:exit'(node) { + if (node.key.type === 'Identifier' && node.key.name === 'execute') { + inPageQueryExecute = false; + console.log(' [MethodDefinition:exit] execute()'); + } + }, + + 'Program:exit'() { + console.log('\n[Program:exit]'); + console.log(' hasManualTransformation:', hasManualTransformation); + console.log(' hasBuilderCall:', hasBuilderCall); + console.log(' Should report?', hasManualTransformation && !hasBuilderCall); + + if (hasManualTransformation && !hasBuilderCall) { + console.log(' ✓✓✓ REPORTING ERROR ✓✓✓'); + context.report({ + node: context.getSourceCode().ast, + messageId: 'mustUseBuilder', + }); + } + }, + }; + }, +}; + +// Test it +const mockContext = { + getSourceCode: () => ({ ast: {} }), + report: (data) => { + console.log('\n*** REPORT CALLED ***'); + } +}; + +const visitor = rule.create(mockContext); + +console.log('=== Test with Logging ===\n'); + +visitor.ClassDeclaration({ + id: { name: 'AdminDashboardPageQuery' } +}); + +visitor.MethodDefinition({ + key: { type: 'Identifier', name: 'execute' }, + parent: { type: 'ClassBody' } +}); + +visitor.VariableDeclarator({ + init: { type: 'CallExpression' } +}); + +visitor.ReturnStatement({ + argument: { + type: 'CallExpression', + callee: { + type: 'MemberExpression', + object: { type: 'Identifier', name: 'Result' }, + property: { type: 'Identifier', name: 'ok' } + }, + arguments: [{ type: 'Identifier', name: 'apiDto' }] + } +}); + +visitor['MethodDefinition:exit']({ + key: { type: 'Identifier', name: 'execute' } +}); + +visitor['Program:exit'](); + +console.log('\n=== Done ==='); diff --git a/apps/website/eslint-rules/test-rule.js b/apps/website/eslint-rules/test-rule.js new file mode 100644 index 000000000..d12b846a9 --- /dev/null +++ b/apps/website/eslint-rules/test-rule.js @@ -0,0 +1,63 @@ +/** + * Test script for page-query-must-use-builders rule + */ + +const rule = require('./page-query-must-use-builders.js'); +const { Linter } = require('eslint'); + +const linter = new Linter(); + +// Register the plugin +linter.defineRule('gridpilot-rules/page-query-must-use-builders', rule); + +const code = ` +import { AdminApiClient } from '@/lib/api/admin/AdminApiClient'; +import { PageQuery } from '@/lib/contracts/page-queries/PageQuery'; +import { Result } from '@/lib/contracts/Result'; +import { EnhancedErrorReporter } from '@/lib/infrastructure/EnhancedErrorReporter'; +import { ConsoleLogger } from '@/lib/infrastructure/logging/ConsoleLogger'; +import { AdminService } from '@/lib/services/admin/AdminService'; +import type { DashboardStats } from '@/lib/api/admin/AdminApiClient'; + +export class AdminDashboardPageQuery implements PageQuery { + async execute(): Promise> { + try { + const logger = new ConsoleLogger(); + const errorReporter = new EnhancedErrorReporter(logger, { + showUserNotifications: false, + logToConsole: true, + reportToExternal: process.env.NODE_ENV === 'production', + }); + + const baseUrl = process.env.NEXT_PUBLIC_API_BASE_URL || 'http://localhost:3001'; + const apiClient = new AdminApiClient(baseUrl, errorReporter, logger); + const adminService = new AdminService(apiClient); + + const apiDto = await adminService.getDashboardStats(); + + return Result.ok(apiDto); + } catch (err) { + console.error('AdminDashboardPageQuery failed:', err); + + if (err instanceof Error && (err.message.includes('403') || err.message.includes('401'))) { + return Result.err('notFound'); + } + + return Result.err('admin_dashboard_fetch_failed'); + } + } +} +`; + +const messages = linter.verify(code, { + parser: '@typescript-eslint/parser', + parserOptions: { + ecmaVersion: 2020, + sourceType: 'module', + }, + rules: { + 'gridpilot-rules/page-query-must-use-builders': 'error', + }, +}); + +console.log('ESLint messages:', messages); diff --git a/apps/website/eslint-rules/view-data-builder-contract.js b/apps/website/eslint-rules/view-data-builder-contract.js index 11e3fa0e0..cd8c154e6 100644 --- a/apps/website/eslint-rules/view-data-builder-contract.js +++ b/apps/website/eslint-rules/view-data-builder-contract.js @@ -1,10 +1,10 @@ /** * ESLint rule to enforce View Data Builder contract - * + * * View Data Builders must: * 1. Be classes named *ViewDataBuilder * 2. Have a static build() method - * 3. Accept an API DTO as parameter + * 3. Accept API DTO as parameter (named 'apiDto', NOT 'pageDto') * 4. Return View Data */ @@ -22,6 +22,7 @@ module.exports = { notAClass: 'View Data Builders must be classes named *ViewDataBuilder', missingBuildMethod: 'View Data Builders must have a static build() method', invalidBuildSignature: 'build() method must accept API DTO and return View Data', + wrongParameterName: 'Parameter must be named "apiDto", not "pageDto" or other names', }, }, @@ -33,6 +34,7 @@ module.exports = { let hasBuildMethod = false; let hasCorrectSignature = false; + let hasCorrectParameterName = false; return { // Check class declaration @@ -47,7 +49,7 @@ module.exports = { } // Check for static build method - const buildMethod = node.body.body.find(member => + const buildMethod = node.body.body.find(member => member.type === 'MethodDefinition' && member.key.type === 'Identifier' && member.key.name === 'build' && @@ -58,10 +60,22 @@ module.exports = { hasBuildMethod = true; // Check signature - should have at least one parameter - if (buildMethod.value && - buildMethod.value.params && + if (buildMethod.value && + buildMethod.value.params && buildMethod.value.params.length > 0) { hasCorrectSignature = true; + + // Check parameter name + const firstParam = buildMethod.value.params[0]; + if (firstParam.type === 'Identifier' && firstParam.name === 'apiDto') { + hasCorrectParameterName = true; + } else if (firstParam.type === 'Identifier' && firstParam.name === 'pageDto') { + // Report specific error for pageDto + context.report({ + node: firstParam, + messageId: 'wrongParameterName', + }); + } } } }, @@ -77,6 +91,12 @@ module.exports = { node: context.getSourceCode().ast, messageId: 'invalidBuildSignature', }); + } else if (!hasCorrectParameterName) { + // Only report if not already reported for pageDto + context.report({ + node: context.getSourceCode().ast, + messageId: 'wrongParameterName', + }); } }, }; diff --git a/apps/website/lib/contracts/mutations/Mutation.ts b/apps/website/lib/contracts/mutations/Mutation.ts index bd0ecfa5c..87d7f49f7 100644 --- a/apps/website/lib/contracts/mutations/Mutation.ts +++ b/apps/website/lib/contracts/mutations/Mutation.ts @@ -5,8 +5,13 @@ import { Result } from "../Result"; * * Purpose: Framework-agnostic write operations * - * Rules: - * - Orchestrates services for writes + * Architecture: + * - Server Action constructs Mutation + * - Mutation constructs Service + * - Service creates own dependencies (API Client, Logger, ErrorReporter) + * - Service returns Result + * - Mutation maps DomainError → MutationError + * - Mutation returns Result * - No HTTP/API calls directly * - No 'use client' directive * - No 'use server' directive @@ -18,18 +23,48 @@ import { Result } from "../Result"; * Pattern: * Server Action → Mutation → Service → API Client * + * Example: + * ```typescript + * export class UpdateUserStatusMutation implements Mutation { + * async execute(input: UpdateUserStatusInput): Promise> { + * const service = new UserService(); + * const result = await service.updateUserStatus(input.userId, input.status); + * + * if (result.isErr()) { + * return Result.err(mapToMutationError(result.error)); + * } + * + * return Result.ok(undefined); + * } + * } + * ``` + * * Design Principle: * Each mutation does ONE thing. If you need multiple operations, * create multiple mutation classes (e.g., UpdateUserStatusMutation, DeleteUserMutation). * This follows the same pattern as Page Queries. + * + * @template TInput - The input type for the mutation + * @template TOutput - The output type on success + * @template TError - The error type (default: string for backward compatibility) */ -export interface Mutation { +export interface Mutation { /** * Execute the mutation * + * Manual construction pattern: + * ```typescript + * const service = new MyService(); + * const result = await service.doWrite(input); + * if (result.isErr()) { + * return Result.err(mapToMutationError(result.error)); + * } + * return Result.ok(undefined); + * ``` + * * @param input - Mutation input - * @returns Result indicating success or error + * @returns Promise> */ - execute(input: TInput): Promise>; + execute(input: TInput): Promise>; } \ No newline at end of file diff --git a/apps/website/lib/contracts/mutations/MutationError.ts b/apps/website/lib/contracts/mutations/MutationError.ts new file mode 100644 index 000000000..bd5927740 --- /dev/null +++ b/apps/website/lib/contracts/mutations/MutationError.ts @@ -0,0 +1,45 @@ +/** + * Mutation Error Type + * + * Errors that can be handled by the client after a mutation. + * These are mapped from DomainErrors by Mutations. + */ + +export type MutationError = + | 'userNotFound' // User doesn't exist + | 'noPermission' // Insufficient permissions + | 'invalidData' // Validation failed + | 'updateFailed' // Update operation failed + | 'deleteFailed' // Delete operation failed + | 'createFailed' // Create operation failed + | 'networkError' // Network/communication error + | 'serverError' // Generic server error + | 'unknown'; // Unknown error + +// Helper to map DomainError to MutationError +export function mapToMutationError(domainError: any): MutationError { + const errorType = domainError?.type || domainError?.name || 'unknown'; + + switch (errorType) { + case 'notFound': + case 'NotFoundError': + case 'userNotFound': + return 'userNotFound'; + case 'unauthorized': + case 'UnauthorizedError': + case 'ForbiddenError': + return 'noPermission'; + case 'validationError': + case 'ValidationError': + return 'invalidData'; + case 'serverError': + case 'ServerError': + case 'HttpServerError': + return 'serverError'; + case 'networkError': + case 'NetworkError': + return 'networkError'; + default: + return 'unknown'; + } +} diff --git a/apps/website/lib/contracts/page-queries/PageQuery.ts b/apps/website/lib/contracts/page-queries/PageQuery.ts index f0ad9087c..887813bc6 100644 --- a/apps/website/lib/contracts/page-queries/PageQuery.ts +++ b/apps/website/lib/contracts/page-queries/PageQuery.ts @@ -2,25 +2,39 @@ import { Result } from "../Result"; /** * PageQuery contract interface - * + * * Defines the canonical contract for all server-side page queries. - * - * Based on WEBSITE_PAGE_QUERIES.md: + * + * Architecture: * - Server-side composition classes - * - Call services that call apps/api - * - Assemble a Page DTO - * - Explicit result describing route outcome + * - Construct Services manually (no DI container) + * - Services create their own dependencies (API Client, Logger, ErrorReporter) + * - Services return Result + * - PageQuery maps DomainError → PresentationError + * - PageQuery returns Result * - Do not implement business rules - * - * @template TApiDto - The API DTO type this query produces + * + * @template TViewData - The ViewData type this query produces for templates * @template TParams - The parameters required to execute this query + * @template TError - The error type (default: string for backward compatibility) */ -export interface PageQuery { +export interface PageQuery { /** * Execute the page query - * + * + * Manual construction pattern: + * ```typescript + * const service = new MyService(); + * const result = await service.getData(); + * if (result.isErr()) { + * return Result.err(mapToPresentationError(result.error)); + * } + * const viewData = MyViewDataBuilder.build(result.value); + * return Result.ok(viewData); + * ``` + * * @param params - Parameters required for query execution - * @returns Promise resolving to a Result + * @returns Promise> */ - execute(params: TParams): Promise>; + execute(params: TParams): Promise>; } diff --git a/apps/website/lib/contracts/page-queries/PresentationError.ts b/apps/website/lib/contracts/page-queries/PresentationError.ts new file mode 100644 index 000000000..c09c08bb9 --- /dev/null +++ b/apps/website/lib/contracts/page-queries/PresentationError.ts @@ -0,0 +1,42 @@ +/** + * Presentation Error Type + * + * Errors that can be handled by the presentation layer (RSC pages, templates). + * These are mapped from DomainErrors by PageQueries. + */ + +export type PresentationError = + | 'notFound' // Resource not found - show 404 page + | 'redirect' // Redirect to another page (e.g., login) + | 'unauthorized' // Not authorized - show access denied + | 'serverError' // Generic server error + | 'networkError' // Network/communication error + | 'validationError' // Invalid input data + | 'unknown'; // Unknown error + +// Helper to map DomainError to PresentationError +export function mapToPresentationError(domainError: any): PresentationError { + const errorType = domainError?.type || domainError?.name || 'unknown'; + + switch (errorType) { + case 'notFound': + case 'NotFoundError': + return 'notFound'; + case 'unauthorized': + case 'UnauthorizedError': + case 'ForbiddenError': + return 'unauthorized'; + case 'serverError': + case 'ServerError': + case 'HttpServerError': + return 'serverError'; + case 'networkError': + case 'NetworkError': + return 'networkError'; + case 'validationError': + case 'ValidationError': + return 'validationError'; + default: + return 'unknown'; + } +} diff --git a/apps/website/lib/contracts/presenters/Presenter.ts b/apps/website/lib/contracts/presenters/Presenter.ts index ffcc50143..6fd4e250a 100644 --- a/apps/website/lib/contracts/presenters/Presenter.ts +++ b/apps/website/lib/contracts/presenters/Presenter.ts @@ -1,6 +1,8 @@ /** * Presenter contract * + * @deprecated Must use builders instead. + * * Pure, deterministic transformation between presentation models. * * Based on PRESENTERS.md: diff --git a/apps/website/lib/contracts/services/Service.ts b/apps/website/lib/contracts/services/Service.ts index ec20e547c..b71a45037 100644 --- a/apps/website/lib/contracts/services/Service.ts +++ b/apps/website/lib/contracts/services/Service.ts @@ -1,37 +1,50 @@ /** - * Service contract - * + * Service Contract + * * Orchestration boundary for server-side operations. - * Returns API DTOs or Page DTOs only. + * Services are self-contained and create their own dependencies. + * Returns Result for type-safe error handling. * Must be stateless. - * + * + * Architecture: + * - Services are self-contained (no constructor parameters) + * - Services create their own dependencies (API Client, Logger, ErrorReporter) + * - Services return Result + * - Services convert HTTP errors to Domain errors + * * Based on WEBSITE_CONTRACT.md: * - Services orchestrate IO and composition * - They do not prepare UI - * - They return ApiDto or PageDto only + * - They return Result */ +import { Result } from '@/lib/contracts/Result'; + /** - * Base service interface for orchestration operations + * Domain error type for services + * Services should define specific error types based on their domain */ -export interface Service { +export type DomainError = + | { type: 'notFound'; message: string } + | { type: 'unauthorized'; message: string } + | { type: 'forbidden'; message: string } + | { type: 'validation'; message: string } + | { type: 'serverError'; message: string } + | { type: 'networkError'; message: string } + | { type: 'unknown'; message: string }; + +/** + * Service interface for orchestration operations + * All service methods must return Result with domain errors + * + * Type Parameters: + * - TApiDto: The API Transport DTO type returned on success + * - TError: The domain error type (defaults to DomainError) + */ +export interface Service { /** * Execute a service operation - * Returns either API Transport DTO or Page DTO + * Returns Result with API DTO or Domain Error */ - execute(...args: unknown[]): Promise; -} - -/** - * Service that returns API Transport DTOs - */ -export interface ApiService extends Service { - execute(...args: unknown[]): Promise; -} - -/** - * Service that returns Page DTOs - */ -export interface PageService extends Service { - execute(...args: unknown[]): Promise; + execute(...args: unknown[]): Promise>; } \ No newline at end of file