Auto merge of #17252 - davidbarsky:david/refactor-standalone-bools-into-struct, r=Veykril

internal: refactor `prefer_no_std`/`prefer_prelude` bools into a struct
I noticed that there's a large number of functions/arguments during an unrelated change that take two booleans and realized they're _probably_ better off being in a single struct—less error-prone, etc.
Feel free to suggest a better name than `ImportPathConfig`/close this entirely! I can also make these args enums; just hopefully making this a little more misuse-resistant.

Author: bors

src/tools/rust-analyzer/crates/hir-def/src/find_path.rs

@@ -1,9 +1,6 @@
 //! An algorithm to find a path to refer to a certain item.
 
-use std::{
- cmp::Ordering,
- iter::{self, once},
-};
+use std::{cmp::Ordering, iter};
 
 use hir_expand::{
  name::{known,AsName,Name},
@@ -17,7 +14,7 @@ use crate::{
  nameres::DefMap,
  path::{ModPath,PathKind},
  visibility::{Visibility,VisibilityExplicitness},
-ModuleDefId,ModuleId,
+ImportPathConfig,ModuleDefId,ModuleId,
 };
 
 /// Find a path that can be used to refer to a certain item. This can depend on
@@ -28,21 +25,10 @@ pub fn find_path(
 from:ModuleId,
 prefix_kind:PrefixKind,
 ignore_local_imports:bool,
-prefer_no_std:bool,
-prefer_prelude:bool,
+cfg:ImportPathConfig,
 ) -> Option<ModPath>{
 let _p = tracing::span!(tracing::Level::INFO,"find_path").entered();
-find_path_inner(
-FindPathCtx{
- db,
-prefix: prefix_kind,
- prefer_no_std,
- prefer_prelude,
- ignore_local_imports,
-},
- item,
- from,
-)
+find_path_inner(FindPathCtx{ db,prefix: prefix_kind, cfg, ignore_local_imports }, item, from)
 }
 
 #[derive(Copy,Clone,Debug)]
@@ -88,16 +74,15 @@ impl PrefixKind {
 structFindPathCtx<'db>{
 db:&'dbdynDefDatabase,
 prefix:PrefixKind,
-prefer_no_std:bool,
-prefer_prelude:bool,
+cfg:ImportPathConfig,
 ignore_local_imports:bool,
 }
 
 /// Attempts to find a path to refer to the given `item` visible from the `from` ModuleId
 fnfind_path_inner(ctx:FindPathCtx<'_>,item:ItemInNs,from:ModuleId) -> Option<ModPath>{
 // - if the item is a builtin, it's in scope
 ifletItemInNs::Types(ModuleDefId::BuiltinType(builtin)) = item {
-returnSome(ModPath::from_segments(PathKind::Plain,once(builtin.as_name())));
+returnSome(ModPath::from_segments(PathKind::Plain,iter::once(builtin.as_name())));
 }
 
 let def_map = from.def_map(ctx.db);
@@ -107,7 +92,11 @@ fn find_path_inner(ctx: FindPathCtx<'_>, item: ItemInNs, from: ModuleId) -> Opti
 letmut visited_modules = FxHashSet::default();
 returnfind_path_for_module(
 FindPathCtx{
-prefer_no_std: ctx.prefer_no_std || ctx.db.crate_supports_no_std(crate_root.krate),
+cfg:ImportPathConfig{
+prefer_no_std: ctx.cfg.prefer_no_std
+ || ctx.db.crate_supports_no_std(crate_root.krate),
+ ..ctx.cfg
+},
  ..ctx
 },
 &def_map,
@@ -132,7 +121,7 @@ fn find_path_inner(ctx: FindPathCtx<'_>, item: ItemInNs, from: ModuleId) -> Opti
 // - if the item is already in scope, return the name under which it is
 let scope_name = find_in_scope(ctx.db,&def_map, from, item, ctx.ignore_local_imports);
 ifletSome(scope_name) = scope_name {
-returnSome(ModPath::from_segments(prefix.path_kind(),Some(scope_name)));
+returnSome(ModPath::from_segments(prefix.path_kind(),iter::once(scope_name)));
 }
 }
 
@@ -160,7 +149,11 @@ fn find_path_inner(ctx: FindPathCtx<'_>, item: ItemInNs, from: ModuleId) -> Opti
 
 calculate_best_path(
 FindPathCtx{
-prefer_no_std: ctx.prefer_no_std || ctx.db.crate_supports_no_std(crate_root.krate),
+cfg:ImportPathConfig{
+prefer_no_std: ctx.cfg.prefer_no_std
+ || ctx.db.crate_supports_no_std(crate_root.krate),
+ ..ctx.cfg
+},
  ..ctx
 },
 &def_map,
@@ -213,7 +206,7 @@ fn find_path_for_module(
 }else{
 PathKind::Plain
 };
-returnSome((ModPath::from_segments(kind,once(name.clone())),Stable));
+returnSome((ModPath::from_segments(kind,iter::once(name.clone())),Stable));
 }
 }
 let prefix = if module_id.is_within_block(){PrefixKind::Plain}else{ ctx.prefix};
@@ -231,7 +224,10 @@ fn find_path_for_module(
 );
 ifletSome(scope_name) = scope_name {
 // - if the item is already in scope, return the name under which it is
-returnSome((ModPath::from_segments(prefix.path_kind(),once(scope_name)),Stable));
+returnSome((
+ModPath::from_segments(prefix.path_kind(), iter::once(scope_name)),
+Stable,
+));
 }
 }
 
@@ -305,7 +301,7 @@ fn find_in_prelude(
 });
 
 if found_and_same_def.unwrap_or(true){
-Some(ModPath::from_segments(PathKind::Plain,once(name.clone())))
+Some(ModPath::from_segments(PathKind::Plain,iter::once(name.clone())))
 }else{
 None
 }
@@ -381,9 +377,7 @@ fn calculate_best_path(
  path.0.push_segment(name);
 
 let new_path = match best_path.take(){
-Some(best_path) => {
-select_best_path(best_path, path, ctx.prefer_no_std, ctx.prefer_prelude)
-}
+Some(best_path) => select_best_path(best_path, path, ctx.cfg),
 None => path,
 };
  best_path_len = new_path.0.len();
@@ -425,12 +419,7 @@ fn calculate_best_path(
 );
 
 let new_path_with_stab = match best_path.take(){
-Some(best_path) => select_best_path(
- best_path,
- path_with_stab,
- ctx.prefer_no_std,
- ctx.prefer_prelude,
-),
+Some(best_path) => select_best_path(best_path, path_with_stab, ctx.cfg),
 None => path_with_stab,
 };
 update_best_path(&mut best_path, new_path_with_stab);
@@ -446,8 +435,7 @@ fn calculate_best_path(
 fnselect_best_path(
  old_path @ (_, old_stability):(ModPath,Stability),
  new_path @ (_, new_stability):(ModPath,Stability),
-prefer_no_std:bool,
-prefer_prelude:bool,
+cfg:ImportPathConfig,
 ) -> (ModPath,Stability){
 match(old_stability, new_stability){
 (Stable,Unstable) => return old_path,
@@ -461,7 +449,7 @@ fn select_best_path(
 let(old_path, _) = &old;
 let new_has_prelude = new_path.segments().iter().any(|seg| seg == &known::prelude);
 let old_has_prelude = old_path.segments().iter().any(|seg| seg == &known::prelude);
-match(new_has_prelude, old_has_prelude, prefer_prelude){
+match(new_has_prelude, old_has_prelude,cfg.prefer_prelude){
 (true,false,true) | (false,true,false) => new,
 (true,false,false) | (false,true,true) => old,
 // no prelude difference in the paths, so pick the shorter one
@@ -482,7 +470,7 @@ fn select_best_path(
 
 match(old_path.0.segments().first(), new_path.0.segments().first()){
 (Some(old),Some(new))ifSTD_CRATES.contains(old) && STD_CRATES.contains(new) => {
-let rank = match prefer_no_std {
+let rank = matchcfg.prefer_no_std{
 false => |name:&Name| match name {
  name if name == &known::core => 0,
  name if name == &known::alloc => 1,
@@ -647,10 +635,9 @@ mod tests {
 {
 let found_path = find_path_inner(
 FindPathCtx{
-prefer_no_std:false,
 db:&db,
  prefix,
- prefer_prelude,
+cfg:ImportPathConfig{prefer_no_std:false,prefer_prelude},
  ignore_local_imports,
 },
  resolved,

src/tools/rust-analyzer/crates/hir-def/src/lib.rs

@@ -108,6 +108,15 @@ use crate::{
 
 typeFxIndexMap<K,V> =
  indexmap::IndexMap<K,V, std::hash::BuildHasherDefault<rustc_hash::FxHasher>>;
+/// A wrapper around two booleans, [`ImportPathConfig::prefer_no_std`] and [`ImportPathConfig::prefer_prelude`].
+#[derive(Debug,Clone,PartialEq,Eq,Hash,Copy)]
+pubstructImportPathConfig{
+/// If true, prefer to unconditionally use imports of the `core` and `alloc` crate
+/// over the std.
+pubprefer_no_std:bool,
+/// If true, prefer import paths containing a prelude module.
+pubprefer_prelude:bool,
+}
 
 #[derive(Debug)]
 pubstructItemLoc<N:ItemTreeNode>{

src/tools/rust-analyzer/crates/hir-ty/src/display.rs

@@ -21,7 +21,8 @@ use hir_def::{
  path::{Path,PathKind},
  type_ref::{TraitBoundModifier,TypeBound,TypeRef},
  visibility::Visibility,
-HasModule,ItemContainerId,LocalFieldId,Lookup,ModuleDefId,ModuleId,TraitId,
+HasModule,ImportPathConfig,ItemContainerId,LocalFieldId,Lookup,ModuleDefId,ModuleId,
+TraitId,
 };
 use hir_expand::name::Name;
 use intern::{Internable,Interned};
@@ -1001,8 +1002,7 @@ impl HirDisplay for Ty {
  module_id,
 PrefixKind::Plain,
 false,
-false,
-true,
+ImportPathConfig{prefer_no_std:false,prefer_prelude:true},
 ){
 write!(f,"{}", path.display(f.db.upcast()))?;
 }else{

src/tools/rust-analyzer/crates/hir/src/lib.rs

@@ -122,6 +122,7 @@ pub use {
  per_ns::Namespace,
  type_ref::{Mutability,TypeRef},
  visibility::Visibility,
+ImportPathConfig,
 // FIXME: This is here since some queries take it as input that are used
 // outside of hir.
 {AdtId,MacroId,ModuleDefId},
@@ -792,17 +793,15 @@ impl Module {
 self,
 db:&dynDefDatabase,
 item:implInto<ItemInNs>,
-prefer_no_std:bool,
-prefer_prelude:bool,
+cfg:ImportPathConfig,
 ) -> Option<ModPath>{
  hir_def::find_path::find_path(
  db,
  item.into().into(),
 self.into(),
 PrefixKind::Plain,
 false,
- prefer_no_std,
- prefer_prelude,
+ cfg,
 )
 }
 
@@ -813,18 +812,9 @@ impl Module {
 db:&dynDefDatabase,
 item:implInto<ItemInNs>,
 prefix_kind:PrefixKind,
-prefer_no_std:bool,
-prefer_prelude:bool,
+cfg:ImportPathConfig,
 ) -> Option<ModPath>{
- hir_def::find_path::find_path(
- db,
- item.into().into(),
-self.into(),
- prefix_kind,
-true,
- prefer_no_std,
- prefer_prelude,
-)
+ hir_def::find_path::find_path(db, item.into().into(),self.into(), prefix_kind,true, cfg)
 }
 }