Make the AST Checker `pub(crate)` (#4498)

crates/ruff/src/checkers/ast/mod.rs

@@ -57,7 +57,7 @@ use crate::{autofix, docstrings, noqa, warn_user};
 
 mod deferred;
 
-pub struct Checker<'a> {
+pub(crate) struct Checker<'a> {
     // Settings, static metadata, etc.
     path: &'a Path,
     module_path: Option<&'a [String]>,
@@ -65,23 +65,23 @@ pub struct Checker<'a> {
     is_stub: bool,
     noqa: flags::Noqa,
     noqa_line_for: &'a NoqaMapping,
-    pub settings: &'a Settings,
+    pub(crate) settings: &'a Settings,
-    pub locator: &'a Locator<'a>,
+    pub(crate) locator: &'a Locator<'a>,
-    pub stylist: &'a Stylist<'a>,
+    pub(crate) stylist: &'a Stylist<'a>,
-    pub indexer: &'a Indexer,
+    pub(crate) indexer: &'a Indexer,
-    pub importer: Importer<'a>,
+    pub(crate) importer: Importer<'a>,
     // Stateful fields.
-    pub ctx: Context<'a>,
+    pub(crate) ctx: Context<'a>,
-    pub diagnostics: Vec<Diagnostic>,
+    pub(crate) diagnostics: Vec<Diagnostic>,
-    pub deletions: FxHashSet<RefEquality<'a, Stmt>>,
+    pub(crate) deletions: FxHashSet<RefEquality<'a, Stmt>>,
     deferred: Deferred<'a>,
     // Check-specific state.
-    pub flake8_bugbear_seen: Vec<&'a Expr>,
+    pub(crate) flake8_bugbear_seen: Vec<&'a Expr>,
 }
 
 impl<'a> Checker<'a> {
     #[allow(clippy::too_many_arguments)]
-    pub fn new(
+    pub(crate) fn new(
         settings: &'a Settings,
         noqa_line_for: &'a NoqaMapping,
         noqa: flags::Noqa,
@@ -117,12 +117,12 @@ impl<'a> Checker<'a> {
 impl<'a> Checker<'a> {
     /// Return `true` if a patch should be generated under the given autofix
     /// `Mode`.
-    pub fn patch(&self, code: Rule) -> bool {
+    pub(crate) fn patch(&self, code: Rule) -> bool {
         self.settings.rules.should_fix(code)
     }
 
     /// Return `true` if a `Rule` is disabled by a `noqa` directive.
-    pub fn rule_is_ignored(&self, code: Rule, offset: TextSize) -> bool {
+    pub(crate) fn rule_is_ignored(&self, code: Rule, offset: TextSize) -> bool {
         // TODO(charlie): `noqa` directives are mostly enforced in `check_lines.rs`.
         // However, in rare cases, we need to check them here. For example, when
         // removing unused imports, we create a single fix that's applied to all
@@ -137,7 +137,7 @@ impl<'a> Checker<'a> {
     }
 
     /// Create a [`Generator`] to generate source code based on the current AST state.
-    pub fn generator(&self) -> Generator {
+    pub(crate) fn generator(&self) -> Generator {
         fn quote_style(context: &Context, locator: &Locator, indexer: &Indexer) -> Option<Quote> {
             if !context.in_f_string() {
                 return None;
@@ -1253,7 +1253,7 @@ where
                                 level,
                                 module,
                                 self.module_path,
-                                &self.settings.flake8_tidy_imports.ban_relative_imports,
+                                self.settings.flake8_tidy_imports.ban_relative_imports,
                             )
                         {
                             self.diagnostics.push(diagnostic);
@@ -4528,7 +4528,7 @@ impl<'a> Checker<'a> {
     }
 
     /// Visit an [`Expr`], and treat it as a type definition.
-    pub fn visit_type_definition(&mut self, expr: &'a Expr) {
+    pub(crate) fn visit_type_definition(&mut self, expr: &'a Expr) {
         let snapshot = self.ctx.flags;
         self.ctx.flags |= ContextFlags::TYPE_DEFINITION;
         self.visit_expr(expr);
@@ -4536,7 +4536,7 @@ impl<'a> Checker<'a> {
     }
 
     /// Visit an [`Expr`], and treat it as _not_ a type definition.
-    pub fn visit_non_type_definition(&mut self, expr: &'a Expr) {
+    pub(crate) fn visit_non_type_definition(&mut self, expr: &'a Expr) {
         let snapshot = self.ctx.flags;
         self.ctx.flags -= ContextFlags::TYPE_DEFINITION;
         self.visit_expr(expr);
@@ -4546,7 +4546,7 @@ impl<'a> Checker<'a> {
     /// Visit an [`Expr`], and treat it as a boolean test. This is useful for detecting whether an
     /// expressions return value is significant, or whether the calling context only relies on
     /// its truthiness.
-    pub fn visit_boolean_test(&mut self, expr: &'a Expr) {
+    pub(crate) fn visit_boolean_test(&mut self, expr: &'a Expr) {
         let snapshot = self.ctx.flags;
         self.ctx.flags |= ContextFlags::BOOLEAN_TEST;
         self.visit_expr(expr);

crates/ruff/src/importer/mod.rs

@@ -14,7 +14,7 @@ use crate::importer::insertion::Insertion;
 
 mod insertion;
 
-pub struct Importer<'a> {
+pub(crate) struct Importer<'a> {
     python_ast: &'a Suite,
     locator: &'a Locator<'a>,
     stylist: &'a Stylist<'a>,
@@ -22,7 +22,11 @@ pub struct Importer<'a> {
 }
 
 impl<'a> Importer<'a> {
-    pub fn new(python_ast: &'a Suite, locator: &'a Locator<'a>, stylist: &'a Stylist<'a>) -> Self {
+    pub(crate) fn new(
+        python_ast: &'a Suite,
+        locator: &'a Locator<'a>,
+        stylist: &'a Stylist<'a>,
+    ) -> Self {
         Self {
             python_ast,
             locator,
@@ -32,7 +36,7 @@ impl<'a> Importer<'a> {
     }
 
     /// Visit a top-level import statement.
-    pub fn visit_import(&mut self, import: &'a Stmt) {
+    pub(crate) fn visit_import(&mut self, import: &'a Stmt) {
         self.ordered_imports.push(import);
     }
 
@@ -49,7 +53,7 @@ impl<'a> Importer<'a> {
     /// If there are no existing imports, the new import will be added at the top
     /// of the file. Otherwise, it will be added after the most recent top-level
     /// import statement.
-    pub fn add_import(&self, import: &AnyImport, at: TextSize) -> Edit {
+    pub(crate) fn add_import(&self, import: &AnyImport, at: TextSize) -> Edit {
         let required_import = import.to_string();
         if let Some(stmt) = self.preceding_import(at) {
             // Insert after the last top-level import.
@@ -64,7 +68,7 @@ impl<'a> Importer<'a> {
 
     /// Return the top-level [`Stmt`] that imports the given module using `Stmt::ImportFrom`
     /// preceding the given position, if any.
-    pub fn find_import_from(&self, module: &str, at: TextSize) -> Option<&Stmt> {
+    pub(crate) fn find_import_from(&self, module: &str, at: TextSize) -> Option<&Stmt> {
         let mut import_from = None;
         for stmt in &self.ordered_imports {
             if stmt.start() >= at {
@@ -87,7 +91,7 @@ impl<'a> Importer<'a> {
     }
 
     /// Add the given member to an existing `Stmt::ImportFrom` statement.
-    pub fn add_member(&self, stmt: &Stmt, member: &str) -> Result<Edit> {
+    pub(crate) fn add_member(&self, stmt: &Stmt, member: &str) -> Result<Edit> {
         let mut tree = match_module(self.locator.slice(stmt.range()))?;
         let import_from = match_import_from(&mut tree)?;
         let aliases = match_aliases(import_from)?;

crates/ruff/src/rules/flake8_tidy_imports/banned_api.rs

@@ -2,11 +2,12 @@ use rustc_hash::FxHashMap;
 use rustpython_parser::ast::{Expr, Ranged};
 use serde::{Deserialize, Serialize};
 
-use crate::checkers::ast::Checker;
 use ruff_diagnostics::{Diagnostic, Violation};
 use ruff_macros::{derive_message_formats, violation, CacheKey};
 use ruff_python_ast::call_path::from_qualified_name;
 
+use crate::checkers::ast::Checker;
+
 pub type Settings = FxHashMap<String, ApiBan>;
 
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, CacheKey)]
@@ -49,7 +50,7 @@ impl Violation for BannedApi {
 }
 
 /// TID251
-pub fn name_is_banned<T>(checker: &mut Checker, name: String, located: &T)
+pub(crate) fn name_is_banned<T>(checker: &mut Checker, name: String, located: &T)
 where
     T: Ranged,
 {
@@ -66,7 +67,7 @@ where
 }
 
 /// TID251
-pub fn name_or_parent_is_banned<T>(checker: &mut Checker, name: &str, located: &T)
+pub(crate) fn name_or_parent_is_banned<T>(checker: &mut Checker, name: &str, located: &T)
 where
     T: Ranged,
 {
@@ -93,7 +94,7 @@ where
 }
 
 /// TID251
-pub fn banned_attribute_access(checker: &mut Checker, expr: &Expr) {
+pub(crate) fn banned_attribute_access(checker: &mut Checker, expr: &Expr) {
     let banned_api = &checker.settings.flake8_tidy_imports.banned_api;
     if let Some((banned_path, ban)) = checker.ctx.resolve_call_path(expr).and_then(|call_path| {
         banned_api

crates/ruff/src/rules/flake8_tidy_imports/relative_imports.rs

@@ -121,25 +121,20 @@ fn fix_banned_relative_import(
 }
 
 /// TID252
-pub fn banned_relative_import(
+pub(crate) fn banned_relative_import(
     checker: &Checker,
     stmt: &Stmt,
     level: Option<u32>,
     module: Option<&str>,
     module_path: Option<&[String]>,
-    strictness: &Strictness,
+    strictness: Strictness,
 ) -> Option<Diagnostic> {
     let strictness_level = match strictness {
         Strictness::All => 0,
         Strictness::Parents => 1,
     };
     if level? > strictness_level {
-        let mut diagnostic = Diagnostic::new(
+        let mut diagnostic = Diagnostic::new(RelativeImports { strictness }, stmt.range());
-            RelativeImports {
-                strictness: *strictness,
-            },
-            stmt.range(),
-        );
         if checker.patch(diagnostic.kind.rule()) {
             if let Some(fix) =
                 fix_banned_relative_import(stmt, level, module, module_path, checker.generator())

scripts/add_rule.py

@@ -76,7 +76,7 @@ def main(*, name: str, prefix: str, code: str, linter: str) -> None:
     contents = rules_mod.read_text()
     parts = contents.split("\n\n")
 
-    new_pub_use = f"pub use {rule_name_snake}::{{{rule_name_snake}, {name}}}"
+    new_pub_use = f"pub(crate) use {rule_name_snake}::{{{rule_name_snake}, {name}}}"
     new_mod = f"mod {rule_name_snake};"
 
     if len(parts) == 2:
@@ -105,6 +105,7 @@ use crate::checkers::ast::Checker;
 
 #[violation]
 pub struct {name};
+
 impl Violation for {name} {{
     #[derive_message_formats]
     fn message(&self) -> String {{
@@ -117,7 +118,7 @@ impl Violation for {name} {{
         fp.write(
             f"""
 /// {prefix}{code}
-pub fn {rule_name_snake}(checker: &mut Checker) {{}}
+pub(crate) fn {rule_name_snake}(checker: &mut Checker) {{}}
 """,
         )