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

feat(lint): implement noPrivateImports #5388

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

arendjr
Copy link
Contributor

@arendjr arendjr commented Mar 18, 2025

Summary

This implements the new rule noPrivateImports based on the previous noPackagePrivateImports (called useImportRestrictions in 1.x).

I've moved the rule to the correctness group, since it seems that repositories that would like to enforce such visibility rules would consider code that imports restricted symbols to be incorrect.

I've made the rule part of the recommended set as well. This was made possible by changing the default visibility to public, meaning that the rule should only report on symbols with explicit visibility restrictions. For symbols with explicit restrictions, it seems sensible that we show diagnostics by default.

Furthermore, by enabling the rule by default, it gives us an opportunity to see the impact of the rule's performance in the real world. If users complain about the rule's behaviour or its performance, we can still remove it from the recommended set before 2.0 is final.

Thanks to @siketyan for helping with the implementation.

Test Plan

Tests added.

@github-actions github-actions bot added A-Core Area: core A-Project Area: project A-Linter Area: linter L-JavaScript Language: JavaScript and super languages labels Mar 18, 2025
@arendjr arendjr marked this pull request as draft March 18, 2025 13:34
@arendjr arendjr force-pushed the no-private-imports branch from f6a62e5 to bf252a7 Compare March 19, 2025 17:53
@github-actions github-actions bot added A-CLI Area: CLI A-Diagnostic Area: diagnostocis labels Mar 19, 2025
Copy link

codspeed-hq bot commented Mar 19, 2025

CodSpeed Performance Report

Merging #5388 will not alter performance

Comparing arendjr:no-private-imports (a1cd8fa) with main (772dcf5)

Summary

✅ 95 untouched benchmarks

@arendjr arendjr changed the title WIP: noPrivateImports feat(lint): implement noPrivateImports Mar 20, 2025
@arendjr arendjr marked this pull request as ready for review March 20, 2025 10:22
@arendjr arendjr requested review from a team March 20, 2025 10:22
@arendjr arendjr force-pushed the no-private-imports branch from bc71c45 to a1cd8fa Compare March 21, 2025 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CLI Area: CLI A-Core Area: core A-Diagnostic Area: diagnostocis A-Linter Area: linter A-Project Area: project L-JavaScript Language: JavaScript and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants