Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to coding-standard 1.2.3 #47449

Merged
merged 3 commits into from
Aug 25, 2024
Merged

Update to coding-standard 1.2.3 #47449

merged 3 commits into from
Aug 25, 2024

Conversation

kesselb
Copy link
Contributor

@kesselb kesselb commented Aug 23, 2024

  • Resolves: unblock dependabot update for coding-standard 1.2.3

Summary

TODO

  • Update OpenApi definitions
  • Make Psalm happy
  • Add SPDX for .git-blame-ignore-revs
  • Update check for expected files
  • Ping release team to ignore .git-blame-ignore-revs

Checklist

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Psalm found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@kesselb kesselb changed the title Update to coding-standard 1.2.2 Update to coding-standard 1.2.3 Aug 23, 2024
@kesselb
Copy link
Contributor Author

kesselb commented Aug 23, 2024

Error: apps/files_versions/appinfo/routes.php:22:1: InvalidScope: Use of $this in non-class context (see https://psalm.dev/013)

False positive (because the file is included). It's popping up now because the /** @var $this \OCP\Route\IRouter */ (invalid) was changed to /** @var \OCP\Route\IRouter $this */ (valid). That needs an psalm baseline update.

Error: lib/private/Route/Router.php:177:7: NoInterfaceProperties: Interfaces cannot have properties (see https://psalm.dev/028)
Error: lib/private/Route/Router.php:180:4: NoInterfaceProperties: Interfaces cannot have properties (see https://psalm.dev/028)
Error: lib/private/Route/Router.php:182:3: NoInterfaceProperties: Interfaces cannot have properties (see https://psalm.dev/028)

I agree with Psalm the code is highly confusing.

@susnux
Copy link
Contributor

susnux commented Aug 24, 2024

ERROR: There were 1 additional files:
.git-blame-ignore-revs
ERROR: Please remove or add those files again or inform the release team about those now files to be included or excluded from the release tar ball.

Should be excluded from release tar ball, no?

@nickvergessen
Copy link
Member

yes should be removed on packaging

Copy link
Member

@st3iny st3iny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@@ -0,0 +1,7 @@
# .git-blame-ignore-revs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL!

@kesselb
Copy link
Contributor Author

kesselb commented Aug 25, 2024

Error: apps/files_versions/appinfo/routes.php:22:1: InvalidScope: Use of $this in non-class context (see https://psalm.dev/013)
Error: lib/private/Route/Router.php:177:7: NoInterfaceProperties: Interfaces cannot have properties (see https://psalm.dev/028)
Error: lib/private/Route/Router.php:180:4: NoInterfaceProperties: Interfaces cannot have properties (see https://psalm.dev/028)
Error: lib/private/Route/Router.php:182:3: NoInterfaceProperties: Interfaces cannot have properties (see https://psalm.dev/028)

The above errors were fixed by changing @var $this \OCP\Route\IRouter to @var $this \OC\Route\Router in core/routes.php.

if (!isset($this->loadedApps['core'])) {
$this->loadedApps['core'] = true;
$this->useCollection('root');
$this->setupRoutes($this->getAttributeRoutes('core'), 'core');
require_once __DIR__ . '/../../../core/routes.php';
// Also add the OCS collection
$collection = $this->getCollection('root.ocs');
$collection->addPrefix('/ocsapp');
$this->root->addCollection($collection);
}
if ($this->loaded) {
$collection = $this->getCollection('ocs');
$collection->addPrefix('/ocs');
$this->root->addCollection($collection);
}
$this->eventLogger->end('route:load:' . $requestedApp);

Psalm resolves the require_once call and assumes, because of the phpdoc annotation, the the class is now IRouter instead of Router and shows a warning that the interface does not have loaded, etc..

@kesselb
Copy link
Contributor Author

kesselb commented Aug 25, 2024

Cypress is having a good day 🥳 🙌

@kesselb kesselb merged commit 5d63215 into master Aug 25, 2024
173 checks passed
@kesselb kesselb deleted the debt/noid/update-codestyle branch August 25, 2024 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants