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

add config for the psalm to check unused variables and parameter #655

Merged
merged 5 commits into from
Oct 1, 2024

Conversation

nabim777
Copy link
Collaborator

@nabim777 nabim777 commented Jun 25, 2024

Description

As of right now, on CI the psalm misses to throw an error when variables and parameters are not being used. Thus, this PR update the error level of psalm to 4 where there is checking of below errors:
AbstractInstantiation
AssignmentToVoid
CircularReference
ConflictingReferenceConstraint
ContinueOutsideLoop
InvalidOverride
InvalidTypeImport
MethodSignatureMismatch
NonVariableReferenceReturn
OverriddenMethodAccess
ParamNameMismatch
ReservedWord
UnhandledMatchCondition
UninitializedProperty

Related Issue or Workpackage

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Updated CHANGELOG.md file

@nabim777 nabim777 self-assigned this Jun 25, 2024
@nabim777 nabim777 force-pushed the CI/check-unused-variables branch 3 times, most recently from 7669268 to a460716 Compare June 26, 2024 07:18
@SagarGi
Copy link
Collaborator

SagarGi commented Jun 28, 2024

Lets put it in level 4 for now. We can make the configuration better in next PR. @nabim777 But create a WP before that.

@nabim777 nabim777 force-pushed the CI/check-unused-variables branch 4 times, most recently from 6c6cf2a to 63374a5 Compare August 1, 2024 07:09
@nabim777 nabim777 marked this pull request as ready for review August 1, 2024 07:11
@nabim777 nabim777 marked this pull request as draft August 1, 2024 09:29
Copy link

github-actions bot commented Aug 8, 2024

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

@nabim777 nabim777 force-pushed the CI/check-unused-variables branch 2 times, most recently from f101cd9 to ba48389 Compare August 10, 2024 09:43
@nabim777 nabim777 force-pushed the CI/check-unused-variables branch 3 times, most recently from a72ce0f to c27fc4c Compare September 20, 2024 09:12
Signed-off-by: nabim777 <nabinalemagar019@gmail.com>
Signed-off-by: nabim777 <nabinalemagar019@gmail.com>
@nabim777 nabim777 marked this pull request as ready for review September 20, 2024 11:20
@nabim777 nabim777 force-pushed the CI/check-unused-variables branch 2 times, most recently from 2e18c20 to b2dad67 Compare September 23, 2024 05:37
@nabim777 nabim777 marked this pull request as draft September 23, 2024 05:40
psalm.xml Outdated Show resolved Hide resolved
tests/acceptance/features/bootstrap/FeatureContext.php Outdated Show resolved Hide resolved
@nabim777 nabim777 force-pushed the CI/check-unused-variables branch 3 times, most recently from 46bdc06 to 3d789be Compare September 23, 2024 07:43
Signed-off-by: nabim777 <nabinalemagar019@gmail.com>
@nabim777 nabim777 force-pushed the CI/check-unused-variables branch 5 times, most recently from ae209a6 to 1e65fcf Compare September 30, 2024 03:56
Signed-off-by: nabim777 <nabinalemagar019@gmail.com>
Signed-off-by: nabim777 <nabinalemagar019@gmail.com>
Copy link

JS Code Coverage

Coverage after merging CI/check-unused-variables into master will be
87.81%
Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
src
   adminSettings.js0%0%0%0%1, 1, 10–19, 2, 20–25, 3–9
   bootstrap.js0%0%0%0%1, 1–7
   dashboard.js0%0%0%0%1, 1, 10–19, 2, 20–25, 3–9
   fileActions.js0%0%0%0%1, 1, 10–17, 2–9
   personalSettings.js0%0%0%0%1, 1, 10–19, 2, 20–25, 3–9
   projectTab.js0%0%0%0%1, 1, 10–19, 2, 20–29, 3, 30–39, 4, 40–49, 5, 50–59, 6, 60–66, 7–9
   reference.js0%0%0%0%1, 1, 10–19, 2, 20–29, 3, 30–39, 4, 40–49, 5, 50–59, 6, 60, 7–9
   utils.js71.43%33.33%50%73.85%10–14, 17–26, 6–9
src/components
   AdminSettings.vue96.16%93.83%95.16%96.52%1033–1036, 1072–1074, 1094–1097, 518–519, 655, 667–669, 681, 681, 681–686, 689–690, 694–695, 698–699, 703–704, 714–719, 779–781, 793–796, 809–811, 996–998
   OAuthConnectButton.vue91.54%75%100%92.98%59–64, 67–71
   PersonalSettings.vue90.16%94.44%85.71%89.88%100, 110–115, 118–127, 99
src/components/admin
   FieldValue.vue100%100%100%100%
   FormHeading.vue100%100%100%100%
   ProjectFolderError.vue100%100%100%100%
   TermsOfServiceUnsigned.vue100%100%100%100%
   TextInput.vue98.83%84.62%100%99.57%115, 124–125
src/components/icons
   ClippyIcon.vue100%100%100%100%
   OpenProjectIcon.vue100%100%100%100%
src/components/settings
   CheckBox.vue100%100%100%100%
   SettingsTitle.vue96.74%85.71%100%97.53%46–48
src/components/tab
   EmptyContent.vue98.90%95%100%99.35%92–93
   SearchInput.vue95.27%92.96%94.74%95.73%134–135, 188, 199–204, 263–265, 281–283, 287–292
   WorkPackage.vue86.10%73.17%93.33%87.46%102–111, 124–126, 137–141, 151–153, 171–177, 215, 215–220, 220, 220–231, 76–77
src/filesPlugin
   filesPlugin.js0%0%0%0%1, 1, 10, 100–104, 11–19, 2, 20–29, 3, 30–39, 4, 40–49, 5, 50–59, 6, 60–69, 7, 70–79, 8, 80–89, 9, 90–99
   filesPluginLessThan28.js0%0%0%0%1, 1, 10–19, 2, 20–29, 3, 30–39, 4, 40–49, 5, 50–59, 6, 60–69, 7, 70–78, 8–9
src/utils
   workpackageHelper.js93.73%93.10%88.89%94.15%18–22, 49, 49–51, 94–99
src/views
   CreateWorkPackageModal.vue94.34%86.54%91.67%95.50%353–355, 358, 459–462, 467–472, 477–482, 488–491, 494, 510, 510, 550–554, 564–566, 585–587, 617–619, 641–643, 652–656
   Dashboard.vue77.40%80.39%61.90%78.01%103–108, 115, 119–120, 125, 128, 131–134, 139–141, 182–188, 194–197, 199–209, 238–246, 259–273, 51, 63, 88–91, 98–99
   LinkMultipleFilesModal.vue100%100%100%100%
   ProjectsTab.vue94.91%94.23%93.33%95.07%113–116, 142, 153–154, 188–198, 246–248

Copy link

PHP Code Coverage

Coverage after merging CI/check-unused-variables into master will be
60.62%
Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
server/apps/integration_openproject/lib
   Capabilities.php0%100%0%0%19, 26–35
server/apps/integration_openproject/lib/AppInfo
   Application.php7.41%100%25%6%101, 103–106, 108, 110, 112, 116–119, 121–132, 134, 137, 141, 145–147, 74–76, 79–83, 85–90, 92–93, 96
server/apps/integration_openproject/lib/BackgroundJob
   RemoveExpiredDirectUploadTokens.php0%100%0%0%42, 44–46, 55–56
server/apps/integration_openproject/lib/Controller
   ConfigController.php64.76%100%50%65.37%135, 152–153, 155, 157–159, 161–164, 167–168, 170, 212, 222, 226–228, 292–296, 406–408, 410–412, 417–420, 461, 549–552, 554–555, 558, 566–570, 581, 595–598, 606–607, 611–614, 628–632, 634–635, 637–653, 670–675, 677–678, 680–682, 685, 687–703, 716–723, 725–728, 730–734, 743–748
   DirectDownloadController.php0%100%0%0%36–38, 53–55, 57–64
   DirectUploadController.php70.83%100%100%70%141–143, 186–188, 199, 203–206, 208, 218, 225, 241–243, 245–246, 249–254, 257, 259, 267–269, 275–277, 285–287, 302–304, 323, 328, 334
   FilesController.php72.95%100%83.33%72.41%181–182, 244, 249–252, 254, 256–269, 272–273, 275–276, 280–283, 286, 292
   OpenProjectAPIController.php85.60%100%80%85.92%139, 180, 204, 230–233, 236–243, 245–249, 251, 263, 272, 290, 299, 369, 371, 421, 423, 443, 445, 492, 494, 520–523, 526–530, 532, 737–741, 96
server/apps/integration_openproject/lib/Dashboard
   OpenProjectWidget.php0%100%0%0%101, 108, 115–116, 118, 120–137, 69–73, 80, 87, 94
server/apps/integration_openproject/lib/Exception
   OpenprojectAvatarErrorException.php100%100%100%100%
   OpenprojectErrorException.php100%100%100%100%
   OpenprojectFileNotUploadedException.php100%100%100%100%
   OpenprojectGroupfolderSetupConflictException.php100%100%100%100%
   OpenprojectResponseException.php100%100%100%100%
   OpenprojectUnauthorizedUserException.php0%100%0%0%16
server/apps/integration_openproject/lib/Listener
   BeforeGroupDeletedListener.php0%100%0%0%48, 56–57, 60–63, 65
   BeforeNodeInsideOpenProjectGroupfilderChangedListener.php0%100%0%0%41–43, 47–50, 52, 54, 57–58, 60, 62–65, 67–70, 72–74
   BeforeUserDeletedListener.php0%100%0%0%48, 55–56, 58–61, 63
   LoadAdditionalScriptsListener.php0%100%0%0%17, 20–21, 24–26, 28
   LoadSidebarScript.php0%100%0%0%100, 102, 104–105, 107, 109, 111, 113–122, 75–91, 96–97, 99
   OpenProjectReferenceListener.php0%100%0%0%53–54, 57–58, 61–62, 64–67
   TermsOfServiceEventListener.php0%100%0%0%59–60, 65–66, 68–69, 71–73, 76–80
   UserChangedListener.php0%100%0%0%52, 59–60, 63–68, 71
server/apps/integration_openproject/lib/Migration
   Version2001Date20221213083550.php0%100%0%0%47, 57–65, 67–75, 77–79, 81
   Version2310Date20230116153411.php0%100%0%0%46, 49–52, 54–79, 81–82, 84
   Version2400Date20230504144300.php0%100%0%0%47, 57–60
   Version2640Date20240628114301.php0%100%0%0%52, 64–66, 69–70, 73
server/apps/integration_openproject/lib/Reference
   WorkPackageReferenceProvider.php51.67%100%25%58.33%102, 108–111, 114–116, 119, 123, 157, 165–166, 174, 52, 59, 66, 73–75
server/apps/integration_openproject/lib/Search
   OpenProjectSearchProvider.php0%100%0%0%103–104, 107–118, 121–122, 124–125, 128–137, 139–143, 66–69, 76, 83, 91, 93, 96
   OpenProjectSearchResultEntry.php100%100%100%100%
server/apps/integration_openproject/lib/Service
   DatabaseService.php42.31%100%60%40.43%100–102, 125–129, 131, 80–93, 95–99
   DirectDownloadService.php88.46%100%100%87.50%65–66, 68
   DirectUploadService.php42.86%100%66.67%40%112, 118, 79–82, 84–92
   OauthService.php28.30%100%40%27.08%104–112, 123–130, 64–65, 79–87, 89–95
   OpenProjectAPIService.php75.63%100%75.93%75.60%1035–1037, 1039–1041, 1044–1048, 1050–1052, 1061–1064, 1067–1069, 1071, 1074–1079, 1083–1084, 1115–1118, 1137–1144, 1153–1154, 1161–1163, 1165–1168, 1172, 1181, 1199, 1201–1204, 1206–1211, 1353, 1365, 1368, 1390, 1393, 1403–1408, 1533–1535, 1537–1538, 1542–1543, 1545, 1547, 180–184, 254, 391–392,

@nabim777 nabim777 marked this pull request as ready for review September 30, 2024 08:32
Copy link
Collaborator

@individual-it individual-it left a comment

Choose a reason for hiding this comment

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

just small suggestions

@@ -552,9 +553,9 @@ public function theDataOfTheOCSResponseShouldMatch(
PyStringNode $schemaString
): void {
$responseAsJson = json_decode($this->response->getBody()->getContents());
$responseAsJson = $responseAsJson->ocs->data;
$_responseAsJson = $responseAsJson->ocs->data;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
$_responseAsJson = $responseAsJson->ocs->data;
$dataOfResponse = $responseAsJson->ocs->data;

Copy link
Collaborator Author

@nabim777 nabim777 Sep 30, 2024

Choose a reason for hiding this comment

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

when we change the varaible name from _responseAsJson to dataOfResponse then we will get the following error:

ERROR: UnusedVariable - tests/acceptance/features/bootstrap/FeatureContext.php:556:3 - $dataOfResponse is never referenced or the value is not used (see https://psalm.dev/077)
		$dataOfResponse = $responseAsJson->ocs->data;

this is due to the psalm configuration where I have added
findUnusedVariablesAndParams="true"

we have to add _ in the variable name so that psalm will not get the error

another options is just using suppress

/** @psalm-suppress UnusedVariable */

JsonAssertions::assertJsonDocumentMatchesSchema(
$responseAsJson,
$_responseAsJson,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
$_responseAsJson,
json_decode($this->response->getBody()->getContents(),

Copy link
Collaborator Author

@nabim777 nabim777 Sep 30, 2024

Choose a reason for hiding this comment

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

same error cames as in the comment
#655 (comment)

Copy link
Collaborator

@SagarGi SagarGi left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@SagarGi SagarGi merged commit 732ce38 into master Oct 1, 2024
15 checks passed
@nabim777 nabim777 deleted the CI/check-unused-variables branch October 1, 2024 04:06
nabim777 added a commit that referenced this pull request Oct 31, 2024
* make psalm error level check to 4

Signed-off-by: nabim777 <nabinalemagar019@gmail.com>

* cs fix

Signed-off-by: nabim777 <nabinalemagar019@gmail.com>

* check for the UndefinedInterfaceMethod psalm

Signed-off-by: nabim777 <nabinalemagar019@gmail.com>

* try for ignoring server code scanning by psalm

Signed-off-by: nabim777 <nabinalemagar019@gmail.com>

* changes fileid type from int to string

Signed-off-by: nabim777 <nabinalemagar019@gmail.com>

---------

Signed-off-by: nabim777 <nabinalemagar019@gmail.com>
individual-it added a commit that referenced this pull request Oct 31, 2024
Backport: add config for the psalm to check unused variables and parameter (#655)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants