diff --git a/lib/Listener/ProductionVersionGuardListener.php b/lib/Listener/ProductionVersionGuardListener.php index aa1752f0..eda08f38 100644 --- a/lib/Listener/ProductionVersionGuardListener.php +++ b/lib/Listener/ProductionVersionGuardListener.php @@ -84,7 +84,9 @@ public function handle(Event $event): void // OR's ObjectUpdatingEvent exposes the new object via getNewObject() // (not getObject() — the two events have different APIs). $entity = $event->getNewObject(); - } else { + } + + if (isset($entity) === false) { return; } diff --git a/lib/Mcp/OpenBuiltToolProvider.php b/lib/Mcp/OpenBuiltToolProvider.php index eda85b16..4ab5fb84 100644 --- a/lib/Mcp/OpenBuiltToolProvider.php +++ b/lib/Mcp/OpenBuiltToolProvider.php @@ -594,7 +594,9 @@ private function handleUpsertSchema(array $args): array if ($existing !== null) { $schema = $schemaMapper->updateFromArray($existing->getId(), $blob); $action = 'updated'; - } else { + } + + if ($existing === null) { $schema = $schemaMapper->createFromArray($blob); $action = 'created'; @@ -718,10 +720,9 @@ private function handleUpsertPage(array $args): array $manifest['pages'] = array_values($pages); $saved = $this->saveVersionManifest(objectService: $objectService, version: $version, manifest: $manifest); + $action = 'created'; if ($replaced === true) { $action = 'updated'; - } else { - $action = 'created'; } return [ @@ -847,10 +848,9 @@ private function handleUpsertMenuItem(array $args): array $label = (string) ($args['label'] ?? ''); $icon = (string) ($args['icon'] ?? ''); $route = (string) ($args['route'] ?? ''); + $order = 100; if (isset($args['order']) === true) { $order = (int) $args['order']; - } else { - $order = 100; } if ($appSlug === '' || $this->isValidSlug(candidate: $appSlug) === false) { @@ -903,10 +903,9 @@ private function handleUpsertMenuItem(array $args): array $manifest['menu'] = array_values($menu); $saved = $this->saveVersionManifest(objectService: $objectService, version: $version, manifest: $manifest); + $action = 'created'; if ($replaced === true) { $action = 'updated'; - } else { - $action = 'created'; } return [ diff --git a/lib/Service/ApplicationCreationService.php b/lib/Service/ApplicationCreationService.php index 2d1f1940..e6f7ae3e 100644 --- a/lib/Service/ApplicationCreationService.php +++ b/lib/Service/ApplicationCreationService.php @@ -130,7 +130,6 @@ public function createApplication(array $payload): string $appSlug = (string) ($payload['slug'] ?? ''); $appName = (string) ($payload['name'] ?? ''); $description = (string) ($payload['description'] ?? ''); - $preset = (string) ($payload['preset'] ?? ''); $versions = $this->resolveVersionChain(payload: $payload); // ---- State tracker for rollback ------------------------------------- @@ -182,10 +181,9 @@ public function createApplication(array $payload): string if ($state['applicationUuid'] === '') { $orphaned = []; $this->rollback(state: $state, orphaned: $orphaned); + $status = 'complete'; if ($orphaned !== []) { $status = 'partial'; - } else { - $status = 'complete'; } throw new WizardCreationException( @@ -244,9 +242,8 @@ public function createApplication(array $payload): string ); $orphaned = []; $this->rollback(state: $state, orphaned: $orphaned); - if ($orphaned === []) { - $status = 'complete'; - } else { + $status = 'complete'; + if ($orphaned !== []) { $status = 'partial'; } @@ -275,9 +272,8 @@ public function createApplication(array $payload): string ); $orphaned = []; $this->rollback(state: $state, orphaned: $orphaned); - if ($orphaned === []) { - $status = 'complete'; - } else { + $status = 'complete'; + if ($orphaned !== []) { $status = 'partial'; } @@ -329,9 +325,8 @@ public function createApplication(array $payload): string ); $orphaned = []; $this->rollback(state: $state, orphaned: $orphaned); - if ($orphaned === []) { - $status = 'complete'; - } else { + $status = 'complete'; + if ($orphaned !== []) { $status = 'partial'; } @@ -379,9 +374,8 @@ public function createApplication(array $payload): string ); $orphaned = []; $this->rollback(state: $state, orphaned: $orphaned); - if ($orphaned === []) { - $status = 'complete'; - } else { + $status = 'complete'; + if ($orphaned !== []) { $status = 'partial'; } @@ -438,7 +432,6 @@ private function validatePayload(array $payload): void } // Validate preset or custom versions. - $preset = (string) ($payload['preset'] ?? ''); $versions = $this->resolveVersionChain(payload: $payload); if ($versions === []) { diff --git a/lib/Service/SlugValidator.php b/lib/Service/SlugValidator.php index d1a2aa2d..2e3a86cf 100644 --- a/lib/Service/SlugValidator.php +++ b/lib/Service/SlugValidator.php @@ -150,12 +150,13 @@ public function validateChainSlugs(array $slugs): array foreach ($slugs as $idx => $slug) { $lower = strtolower((string) $slug); - if (isset($seen[$lower]) === true) { - $dupes[$lower][] = $seen[$lower]; - $dupes[$lower][] = $idx; - } else { + if (isset($seen[$lower]) === false) { $seen[$lower] = $idx; + continue; } + + $dupes[$lower][] = $seen[$lower]; + $dupes[$lower][] = $idx; } if ($dupes === []) { diff --git a/phpcs-custom-sniffs/CustomSniffs/Sniffs/Commenting/SpecTagSniff.php b/phpcs-custom-sniffs/CustomSniffs/Sniffs/Commenting/SpecTagSniff.php new file mode 100644 index 00000000..ae6ad7ee --- /dev/null +++ b/phpcs-custom-sniffs/CustomSniffs/Sniffs/Commenting/SpecTagSniff.php @@ -0,0 +1,377 @@ + + */ + private const MAGIC_METHODS = [ + '__construct', + '__destruct', + '__get', + '__set', + '__call', + '__callstatic', + '__isset', + '__unset', + '__tostring', + '__invoke', + '__clone', + '__sleep', + '__wakeup', + '__serialize', + '__unserialize', + '__set_state', + '__debuginfo', + ]; + + + /** + * Returns tokens this sniff listens for. + * + * @return array + */ + public function register(): array + { + return [T_CLASS, T_FUNCTION]; + + }//end register() + + + /** + * Process a T_CLASS or T_FUNCTION token. + * + * @param File $phpcsFile The file being scanned. + * @param int $stackPtr Position of the token. + * + * @return void + */ + public function process(File $phpcsFile, $stackPtr): void + { + // Skip test files. + if ($this->isTestFile(phpcsFile: $phpcsFile) === true) { + return; + } + + $tokens = $phpcsFile->getTokens(); + $code = $tokens[$stackPtr]['code']; + + if ($code === T_CLASS) { + $this->processClass(phpcsFile: $phpcsFile, stackPtr: $stackPtr); + return; + } + + if ($code === T_FUNCTION) { + $this->processFunction(phpcsFile: $phpcsFile, stackPtr: $stackPtr); + return; + } + + }//end process() + + + /** + * Check a class declaration for an @spec docblock tag. + * + * Skips anonymous classes (no name follows the T_CLASS keyword). + * + * @param File $phpcsFile The file being scanned. + * @param int $stackPtr Position of the T_CLASS token. + * + * @return void + */ + private function processClass(File $phpcsFile, int $stackPtr): void + { + $tokens = $phpcsFile->getTokens(); + + // Anonymous classes — $var = new class { ... } — have no name; skip. + $namePtr = $phpcsFile->findNext(T_STRING, ($stackPtr + 1), null, false, null, true); + if ($namePtr === false) { + return; + } + + // Sanity: name should be on the same line or within a short window. + $openBracePtr = $phpcsFile->findNext(T_OPEN_CURLY_BRACKET, ($stackPtr + 1)); + if ($openBracePtr !== false && $namePtr > $openBracePtr) { + return; + } + + $className = $tokens[$namePtr]['content']; + + if ($this->hasSpecTag(phpcsFile: $phpcsFile, stackPtr: $stackPtr) === true) { + return; + } + + $message = 'Class %s is missing @spec PHPDoc tag — link back to openspec/changes/{name}/tasks.md#task-N'; + $phpcsFile->addWarning($message, $stackPtr, 'MissingClassSpec', [$className]); + + }//end processClass() + + + /** + * Check a function declaration for an @spec docblock tag. + * + * Only flags public methods declared inside a class. Global functions, + * private/protected methods, and magic methods are skipped. + * + * @param File $phpcsFile The file being scanned. + * @param int $stackPtr Position of the T_FUNCTION token. + * + * @return void + */ + private function processFunction(File $phpcsFile, int $stackPtr): void + { + $tokens = $phpcsFile->getTokens(); + + // Must be inside a class scope. + $className = $this->getEnclosingClassName(phpcsFile: $phpcsFile, stackPtr: $stackPtr); + if ($className === null) { + return; + } + + // Get method name. + $namePtr = $phpcsFile->findNext(T_STRING, ($stackPtr + 1)); + if ($namePtr === false) { + return; + } + + $methodName = $tokens[$namePtr]['content']; + + // Skip magic methods. + if (in_array(strtolower($methodName), self::MAGIC_METHODS, true) === true) { + return; + } + + // Determine visibility: default is public when no modifier present. + if ($this->isPublicMethod(phpcsFile: $phpcsFile, stackPtr: $stackPtr) === false) { + return; + } + + if ($this->hasSpecTag(phpcsFile: $phpcsFile, stackPtr: $stackPtr) === true) { + return; + } + + $message = 'Public method %s::%s() is missing @spec PHPDoc tag'; + $phpcsFile->addWarning($message, $stackPtr, 'MissingMethodSpec', [$className, $methodName]); + + }//end processFunction() + + + /** + * Check whether the docblock directly preceding $stackPtr contains an @spec tag. + * + * Walks backwards from the token skipping whitespace, attribute tokens, and + * visibility/abstract/final/static modifiers. If the next non-skipped token + * is the close of a doc comment, scan the block for @spec. + * + * @param File $phpcsFile The file being scanned. + * @param int $stackPtr Position of the class/function token. + * + * @return bool True when an @spec tag is present. + */ + private function hasSpecTag(File $phpcsFile, int $stackPtr): bool + { + $tokens = $phpcsFile->getTokens(); + + $skip = [ + T_WHITESPACE, + T_ABSTRACT, + T_FINAL, + T_STATIC, + T_PUBLIC, + T_PROTECTED, + T_PRIVATE, + T_READONLY, + T_ATTRIBUTE, + T_ATTRIBUTE_END, + ]; + + $ptr = ($stackPtr - 1); + while ($ptr >= 0) { + $code = $tokens[$ptr]['code']; + + // Skip over attribute blocks (PHP 8 #[Attribute]) in full. + if ($code === T_ATTRIBUTE_END && isset($tokens[$ptr]['attribute_opener']) === true) { + $ptr = ($tokens[$ptr]['attribute_opener'] - 1); + continue; + } + + if (in_array($code, $skip, true) === true) { + $ptr--; + continue; + } + + break; + } + + if ($ptr < 0) { + return false; + } + + if ($tokens[$ptr]['code'] !== T_DOC_COMMENT_CLOSE_TAG) { + return false; + } + + if (isset($tokens[$ptr]['comment_opener']) === false) { + return false; + } + + $opener = $tokens[$ptr]['comment_opener']; + for ($i = $opener; $i <= $ptr; $i++) { + if ($tokens[$i]['code'] === T_DOC_COMMENT_TAG + && strtolower($tokens[$i]['content']) === '@spec' + ) { + return true; + } + } + + return false; + + }//end hasSpecTag() + + + /** + * Determine if the function at $stackPtr is a public method. + * + * Methods default to public when no visibility modifier is present. + * + * @param File $phpcsFile The file being scanned. + * @param int $stackPtr Position of the T_FUNCTION token. + * + * @return bool True when the method is public (explicit or default). + */ + private function isPublicMethod(File $phpcsFile, int $stackPtr): bool + { + $tokens = $phpcsFile->getTokens(); + + $ptr = ($stackPtr - 1); + while ($ptr >= 0) { + $code = $tokens[$ptr]['code']; + if ($code === T_PUBLIC) { + return true; + } + + if ($code === T_PROTECTED || $code === T_PRIVATE) { + return false; + } + + if ($code === T_WHITESPACE + || $code === T_ABSTRACT + || $code === T_FINAL + || $code === T_STATIC + || $code === T_READONLY + ) { + $ptr--; + continue; + } + + // Skip attributes in full. + if ($code === T_ATTRIBUTE_END && isset($tokens[$ptr]['attribute_opener']) === true) { + $ptr = ($tokens[$ptr]['attribute_opener'] - 1); + continue; + } + + if ($code === T_DOC_COMMENT_CLOSE_TAG + || $code === T_COMMENT + || $code === T_OPEN_CURLY_BRACKET + || $code === T_CLOSE_CURLY_BRACKET + || $code === T_SEMICOLON + ) { + // No visibility modifier found — default public. + return true; + } + + $ptr--; + } + + return true; + + }//end isPublicMethod() + + + /** + * Return the name of the class/interface/trait/enum enclosing $stackPtr, or null. + * + * @param File $phpcsFile The file being scanned. + * @param int $stackPtr Position of the token to inspect. + * + * @return string|null The enclosing class name, or null when at file scope. + */ + private function getEnclosingClassName(File $phpcsFile, int $stackPtr): ?string + { + $tokens = $phpcsFile->getTokens(); + + if (isset($tokens[$stackPtr]['conditions']) === false) { + return null; + } + + // Walk the conditions chain looking for the innermost class-like scope. + $classLike = [T_CLASS, T_INTERFACE, T_TRAIT, T_ENUM, T_ANON_CLASS]; + + foreach (array_reverse($tokens[$stackPtr]['conditions'], true) as $scopePtr => $scopeCode) { + if (in_array($scopeCode, $classLike, true) === true) { + $namePtr = $phpcsFile->findNext(T_STRING, ($scopePtr + 1)); + if ($namePtr === false) { + return '{anonymous}'; + } + + // Sanity: ensure the name is before the opening brace for that class. + if (isset($tokens[$scopePtr]['scope_opener']) === true + && $namePtr > $tokens[$scopePtr]['scope_opener'] + ) { + return '{anonymous}'; + } + + return $tokens[$namePtr]['content']; + } + } + + return null; + + }//end getEnclosingClassName() + + + /** + * Check whether the currently-scanned file is a test file. + * + * @param File $phpcsFile The file being scanned. + * + * @return bool True for files under /tests/ or /Tests/. + */ + private function isTestFile(File $phpcsFile): bool + { + $path = str_replace('\\', '/', $phpcsFile->getFilename()); + return (stripos($path, '/tests/') !== false); + + }//end isTestFile() + + +}//end class diff --git a/phpcs-custom-sniffs/CustomSniffs/Sniffs/Nextcloud/NoLegacyServerAccessorsSniff.php b/phpcs-custom-sniffs/CustomSniffs/Sniffs/Nextcloud/NoLegacyServerAccessorsSniff.php new file mode 100644 index 00000000..7697eae1 --- /dev/null +++ b/phpcs-custom-sniffs/CustomSniffs/Sniffs/Nextcloud/NoLegacyServerAccessorsSniff.php @@ -0,0 +1,180 @@ +getDatabaseConnection() + * \OC::$server->getSystemConfig() + * \OC::$server->getLogger() + * + * These named accessors were removed in Nextcloud 34. The replacement pattern + * is constructor dependency injection of the equivalent OCP interface. + * + * PSR-11 lookups such as \OC::$server->get(SomeClass::class) are NOT flagged + * here; service-locator deprecation is tracked separately (design.md, D4). + * + * @author Conduction + * @package CustomSniffs + */ + +namespace CustomSniffs\Sniffs\Nextcloud; + +use PHP_CodeSniffer\Sniffs\Sniff; +use PHP_CodeSniffer\Files\File; + +/** + * NoLegacyServerAccessorsSniff — forbids removed \OC::$server->getX() accessors. + */ +class NoLegacyServerAccessorsSniff implements Sniff +{ + + + /** + * Map of known named accessors to their approved OCP replacement interface. + * + * Covers the accessors that still appeared in this codebase plus the most + * frequently used Nextcloud 34 removals. The error message interpolates the + * accessor name and the replacement from this table so engineers see the + * intended DI target at the violation site. + * + * @var array + */ + private const REPLACEMENTS = [ + 'getSystemConfig' => '\OCP\IConfig', + 'getConfig' => '\OCP\IConfig', + 'getDatabaseConnection' => '\OCP\IDBConnection', + 'getLogger' => '\Psr\Log\LoggerInterface', + 'getL10NFactory' => '\OCP\L10N\IFactory', + 'getL10N' => '\OCP\IL10N (via \OCP\L10N\IFactory)', + 'getUserSession' => '\OCP\IUserSession', + 'getUserManager' => '\OCP\IUserManager', + 'getGroupManager' => '\OCP\IGroupManager', + 'getURLGenerator' => '\OCP\IURLGenerator', + 'getRequest' => '\OCP\IRequest', + 'getRootFolder' => '\OCP\Files\IRootFolder', + 'getAppManager' => '\OCP\App\IAppManager', + 'getSession' => '\OCP\ISession', + 'getMemCacheFactory' => '\OCP\ICacheFactory', + 'getEventDispatcher' => '\OCP\EventDispatcher\IEventDispatcher', + 'getNotificationManager' => '\OCP\Notification\IManager', + 'getTempManager' => '\OCP\ITempManager', + 'getMimeTypeDetector' => '\OCP\Files\IMimeTypeDetector', + 'getMimeTypeLoader' => '\OCP\Files\IMimeTypeLoader', + 'getActivityManager' => '\OCP\Activity\IManager', + 'getDateTimeFormatter' => '\OCP\IDateTimeFormatter', + 'getDateTimeZone' => '\OCP\IDateTimeZone', + 'getTrustedDomainHelper' => '\OCP\Security\ITrustedDomainHelper', + 'getRegisteredAppContainer' => 'explicit constructor injection of the specific service', + ]; + + /** + * Returns tokens this sniff listens for. + * + * Anchors on T_DOUBLE_COLON so we can reconstruct the full pattern + * \OC :: $server -> getX ( in a single process() call. + * + * @return array + */ + public function register(): array + { + return [T_DOUBLE_COLON]; + + }//end register() + + /** + * Process a T_DOUBLE_COLON token — flag if part of \OC::$server->getX(). + * + * @param File $phpcsFile The file being scanned. + * @param int $stackPtr Position of the T_DOUBLE_COLON token. + * + * @return void + */ + public function process(File $phpcsFile, $stackPtr): void + { + $tokens = $phpcsFile->getTokens(); + + // Previous non-whitespace token must be T_STRING "OC". + $prev = $phpcsFile->findPrevious( + types: [T_WHITESPACE], + start: ($stackPtr - 1), + end: null, + exclude: true + ); + if ($prev === false + || $tokens[$prev]['code'] !== T_STRING + || $tokens[$prev]['content'] !== 'OC' + ) { + return; + } + + // Next non-whitespace token must be T_VARIABLE "$server". + $afterColon = $phpcsFile->findNext( + types: [T_WHITESPACE], + start: ($stackPtr + 1), + end: null, + exclude: true + ); + if ($afterColon === false + || $tokens[$afterColon]['code'] !== T_VARIABLE + || $tokens[$afterColon]['content'] !== '$server' + ) { + return; + } + + // Expect T_OBJECT_OPERATOR '->'. + $arrow = $phpcsFile->findNext( + types: [T_WHITESPACE], + start: ($afterColon + 1), + end: null, + exclude: true + ); + if ($arrow === false || $tokens[$arrow]['code'] !== T_OBJECT_OPERATOR) { + return; + } + + // Expect T_STRING method name. + $methodPtr = $phpcsFile->findNext( + types: [T_WHITESPACE], + start: ($arrow + 1), + end: null, + exclude: true + ); + if ($methodPtr === false || $tokens[$methodPtr]['code'] !== T_STRING) { + return; + } + + // Must be followed by ( to be a call. + $openParen = $phpcsFile->findNext( + types: [T_WHITESPACE], + start: ($methodPtr + 1), + end: null, + exclude: true + ); + if ($openParen === false || $tokens[$openParen]['code'] !== T_OPEN_PARENTHESIS) { + return; + } + + $methodName = $tokens[$methodPtr]['content']; + + // PSR-11 ->get(...) is deferred (D4 in design.md) — not flagged here. + if ($methodName === 'get') { + return; + } + + // Only flag named accessors: getX where X starts with an uppercase letter. + if (preg_match(pattern: '/^get[A-Z]/', subject: $methodName) !== 1) { + return; + } + + $replacement = self::REPLACEMENTS[$methodName] ?? 'the corresponding OCP interface'; + + $error = 'Named accessor \\OC::$server->%s() is removed in Nextcloud 34. Inject %s via the constructor instead.'; + $phpcsFile->addError( + $error, + $stackPtr, + 'LegacyNamedAccessor', + [$methodName, $replacement] + ); + + }//end process() +}//end class diff --git a/phpcs.xml b/phpcs.xml index 8e5eb443..c29a227e 100644 --- a/phpcs.xml +++ b/phpcs.xml @@ -6,12 +6,16 @@ */vendor/* + */vendor-bin/* */node_modules/* composer-setup.php - + lib/Resources/template/* + + + @@ -216,4 +220,15 @@ error + + + error + + + + + warning + + diff --git a/phpmd.xml b/phpmd.xml index 11f7d5bc..b9ea3613 100644 --- a/phpmd.xml +++ b/phpmd.xml @@ -9,10 +9,6 @@ This is a custom ruleset for OpenBuilt Nextcloud. - - */lib/Resources/template/* - @@ -27,42 +23,13 @@ + - - - - - - - - - - - - + @@ -80,20 +47,7 @@ - - - - - - + diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon new file mode 100644 index 00000000..80ce68e8 --- /dev/null +++ b/phpstan-baseline.neon @@ -0,0 +1,15 @@ +# PHPStan baseline — captures architectural debt that exists at the time of the +# canonical-root-config sync (Phase 2 fleet rollout). Tracked in +# https://github.com/ConductionNL/openbuilt/issues/126 — fix and remove entries +# as work lands. +parameters: + ignoreErrors: + - + message: "#^Constant OCA\\\\OpenBuilt\\\\Repair\\\\MigrateToVersionedModel\\:\\:VERSIONED_SCHEMA is unused\\.$#" + count: 1 + path: lib/Repair/MigrateToVersionedModel.php + + - + message: "#^Access to static property \\$SERVERROOT on an unknown class OC\\.$#" + count: 1 + path: lib/Service/IconService.php diff --git a/phpstan-bootstrap.php b/phpstan-bootstrap.php index 4c32b41b..b473c89c 100644 --- a/phpstan-bootstrap.php +++ b/phpstan-bootstrap.php @@ -1,5 +1,4 @@ - @@ -54,12 +53,6 @@ - - - - - - @@ -67,31 +60,29 @@ + + + - + - + + - + - - - - - - - - - + + + - - - + + +