[Refactor:CourseMaterials] Phpstan 2.1.32 compatibility#12150
Merged
Conversation
…lderPathsR method
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12150 +/- ##
=========================================
Coverage 21.69% 21.69%
Complexity 9618 9618
=========================================
Files 268 268
Lines 36113 36113
Branches 478 478
=========================================
Hits 7835 7835
Misses 27804 27804
Partials 474 474
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
skara9
requested changes
Oct 21, 2025
| * | ||
| * @param array<mixed> $course_materials - Dictionary: path name => CourseMaterial. | ||
| * @param array<string> $folder_paths - List we append | ||
| * @param array<int|string, int|string> $folder_paths - List we append |
Contributor
There was a problem hiding this comment.
Why do we need to allow integers in folder_paths?
And why are you changing it from a string array to a map?
RyanStyron
requested changes
Nov 18, 2025
skara9
reviewed
Dec 4, 2025
| * @param array<mixed> $course_materials - Dictionary: path name => CourseMaterial. | ||
| * @return array<string> List of folders paths. | ||
| * @param array<string, CourseMaterial|array<string, CourseMaterial>> $course_materials - Dictionary: path name => CourseMaterial. | ||
| * @return array<int, string> List of folders paths. |
Contributor
There was a problem hiding this comment.
Suggested change
| * @return array<int, string> List of folders paths. | |
| * @return array<string> List of folders paths. |
Is there a reason this was changed?
Contributor
There was a problem hiding this comment.
compileAllFolderPaths returns $folder_paths, which is assigned by reference in compileAllFolderPathsR. In compileAllFolderPathsR, $folder_paths is assigned via invocation of array_push, which adds string elements (folder paths) to the array using int keys, which results in a sequential indexed array of strings (array<int, string>). This is the doc type expected by PHPStan, so that's why the change had to be made.
RyanStyron
approved these changes
Dec 15, 2025
skara9
approved these changes
Dec 15, 2025
williamjallen
added a commit
that referenced
this pull request
Jan 3, 2026
) ### Why is this Change Important & Necessary? <!-- Include any GitHub issue that is fixed/closed using "Fixes #<number>" or "Closes #<number>" syntax. Alternately write "Partially addresses #<number>" or "Related to #<number>" as appropriate. --> As a continuation of #12150, before merging #12260, it is necessary to ensure that all of the code is compatible with the new version of PHPStan. ### What is the New Behavior? <!-- Include before & after screenshots/videos if the user interface has changed. --> PHPStan 2.1.32 check now passes after this change. ### What steps should a reviewer take to reproduce or test the bug or new feature? 1. Checkout #12260 and then PHPStan ([documentation](https://submitty.org/developer/testing/linting_static_analysis)) 2. Run `git diff main phpstan-upgrade-prep-2 | git apply` to apply the changes from this branch 3. Run PHPStan again and the test will pass --------- Co-authored-by: William Allen <william.j.allen24@gmail.com>
Aaditya-2407
pushed a commit
to Aaditya-2407/Submitty
that referenced
this pull request
Jan 5, 2026
…mitty#12284) ### Why is this Change Important & Necessary? <!-- Include any GitHub issue that is fixed/closed using "Fixes #<number>" or "Closes #<number>" syntax. Alternately write "Partially addresses #<number>" or "Related to #<number>" as appropriate. --> As a continuation of Submitty#12150, before merging Submitty#12260, it is necessary to ensure that all of the code is compatible with the new version of PHPStan. ### What is the New Behavior? <!-- Include before & after screenshots/videos if the user interface has changed. --> PHPStan 2.1.32 check now passes after this change. ### What steps should a reviewer take to reproduce or test the bug or new feature? 1. Checkout Submitty#12260 and then PHPStan ([documentation](https://submitty.org/developer/testing/linting_static_analysis)) 2. Run `git diff main phpstan-upgrade-prep-2 | git apply` to apply the changes from this branch 3. Run PHPStan again and the test will pass --------- Co-authored-by: William Allen <william.j.allen24@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why is this Change Important & Necessary?
Before we can merge #12093 (superseded by #12260), we need to make sure that all of our code is compatible with the new version of phpstan, and this was the one place that our code has started failing.
What is the New Behavior?
Phpstan 2.1.32 now passes after this change.
What steps should a reviewer take to reproduce or test the bug or new feature?
composer installand then phpstan and view that 1 error is detectedgit diff main phpstan-prep | git applyto get the changes from this branchAutomated Testing & Documentation
Other information