chore: ensure imports are unused before removing#1030
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a safeguard to remove an import only when it is confirmed to be unused, helping avoid accidental deletion of shared imports. Key changes include:
- Addition of a new helper function (UsesImport) with accompanying test cases in utils.go and utils_test.go.
- Updates to the RemoveImport action to conditionally skip removal if the import is still used.
- A reordering of modifications in the package_make_command_stubs.go file to reflect the new logic.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/modify/utils_test.go | Added tests for the new UsesImport function. |
| packages/modify/utils.go | Introduced the UsesImport helper function for checking import usage. |
| packages/modify/actions_test.go | Added test cases to cover scenarios for removing imports. |
| packages/modify/actions.go | Updated RemoveImport to conditionally remove imports based on usage. |
| foundation/console/package_make_command_stubs.go | Reordered modification calls for providers and imports removal. |
| @@ -144,8 +144,8 @@ func main() { | |||
| ). | |||
| Uninstall( | |||
There was a problem hiding this comment.
[nitpick] The reordering of the modification calls for removing providers and imports may affect the intended side effects. Consider adding an inline comment to clarify that this new order maintains the original behavior.
| Uninstall( | |
| Uninstall( | |
| // The order of operations here (removing providers before imports) is intentional | |
| // and does not affect the original behavior. Providers are removed first to ensure | |
| // that any dependencies on the import are handled before the import is removed. |
There was a problem hiding this comment.
Note: The order of removing references before imports is intentional and should be documented.
It helps prevent accidentally removing imports that are still in use. @hwbrzzl
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1030 +/- ##
==========================================
+ Coverage 70.35% 70.40% +0.05%
==========================================
Files 176 176
Lines 12297 12318 +21
==========================================
+ Hits 8651 8672 +21
Misses 3265 3265
Partials 381 381 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
📑 Description
This change adds a check to ensure that an import is actually unused before it is removed from the source file.
It prevents accidentally deleting shared imports that are still required by other packages.
For example, multiple filesystem drivers like
github.com/goravel/cloudinaryandgithub.com/goravel/miniomay each importgithub.com/goravel/framework/contracts/filesystem. When both are installed, this import is shared. If one of them is uninstalled, we should not remove the shared import unless it is truly unused.This fix avoids such cases by checking whether the import alias is still referenced in the AST before removing it.
✅ Checks