Skip to content

Commit 514ff11

Browse files
bors[bot]SomeoneToIgnore
bors[bot]
and
SomeoneToIgnore
authored
Merge #3017
3017: Normalize dashes in crate names r=matklad a=SomeoneToIgnore A follow-up for https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0/topic/Yet.20another.20auto.20import.20bug In theory, I could have used the same new type in the `Dependency` `name` field, but since the `add_dep` method that actually adds a dependency is private, it seems like an unnecessary change now. Co-authored-by: Kirill Bulatov <[email protected]>
2 parents c9e1aab + 78092c7 commit 514ff11

File tree

5 files changed

+84
-27
lines changed

5 files changed

+84
-27
lines changed

‎crates/ra_db/src/fixture.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ use rustc_hash::FxHashMap;
88
use test_utils::{extract_offset, parse_fixture, CURSOR_MARKER};
99

1010
use crate::{
11-
CrateGraph, CrateId, Edition, Env, FileId, FilePosition, RelativePathBuf, SourceDatabaseExt,
12-
SourceRoot, SourceRootId,
11+
input::CrateName, CrateGraph, CrateId, Edition, Env, FileId, FilePosition, RelativePathBuf,
12+
SourceDatabaseExt, SourceRoot, SourceRootId,
1313
};
1414

1515
pub const WORKSPACE: SourceRootId = SourceRootId(0);
@@ -139,7 +139,7 @@ fn with_files(db: &mut dyn SourceDatabaseExt, fixture: &str) -> Option<FilePosit
139139
for (from, to) in crate_deps {
140140
let from_id = crates[&from];
141141
let to_id = crates[&to];
142-
crate_graph.add_dep(from_id, to.into(), to_id).unwrap();
142+
crate_graph.add_dep(from_id, CrateName::new(&to).unwrap(), to_id).unwrap();
143143
}
144144
}
145145

‎crates/ra_db/src/input.rs

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,26 @@ pub struct CrateGraph {
8383
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
8484
pub struct CrateId(pub u32);
8585

86+
pub struct CrateName(SmolStr);
87+
88+
impl CrateName {
89+
/// Crates a crate name, checking for dashes in the string provided.
90+
/// Dashes are not allowed in the crate names,
91+
/// hence the input string is returned as `Err` for those cases.
92+
pub fn new(name: &str) -> Result<CrateName, &str> {
93+
if name.contains('-') {
94+
Err(name)
95+
} else {
96+
Ok(Self(SmolStr::new(name)))
97+
}
98+
}
99+
100+
/// Crates a crate name, unconditionally replacing the dashes with underscores.
101+
pub fn normalize_dashes(name: &str) -> CrateName {
102+
Self(SmolStr::new(name.replace('-', "_")))
103+
}
104+
}
105+
86106
#[derive(Debug, Clone, PartialEq, Eq)]
87107
struct CrateData {
88108
file_id: FileId,
@@ -131,13 +151,13 @@ impl CrateGraph {
131151
pub fn add_dep(
132152
&mut self,
133153
from: CrateId,
134-
name: SmolStr,
154+
name: CrateName,
135155
to: CrateId,
136156
) -> Result<(), CyclicDependenciesError> {
137157
if self.dfs_find(from, to, &mut FxHashSet::default()) {
138158
return Err(CyclicDependenciesError);
139159
}
140-
self.arena.get_mut(&from).unwrap().add_dep(name, to);
160+
self.arena.get_mut(&from).unwrap().add_dep(name.0, to);
141161
Ok(())
142162
}
143163

@@ -268,7 +288,7 @@ pub struct CyclicDependenciesError;
268288

269289
#[cfg(test)]
270290
mod tests {
271-
use super::{CfgOptions, CrateGraph, Edition::Edition2018, Env, FileId, SmolStr};
291+
use super::{CfgOptions, CrateGraph, CrateName, Dependency, Edition::Edition2018, Env, FileId};
272292

273293
#[test]
274294
fn it_should_panic_because_of_cycle_dependencies() {
@@ -279,9 +299,9 @@ mod tests {
279299
graph.add_crate_root(FileId(2u32), Edition2018, CfgOptions::default(), Env::default());
280300
let crate3 =
281301
graph.add_crate_root(FileId(3u32), Edition2018, CfgOptions::default(), Env::default());
282-
assert!(graph.add_dep(crate1, SmolStr::new("crate2"), crate2).is_ok());
283-
assert!(graph.add_dep(crate2, SmolStr::new("crate3"), crate3).is_ok());
284-
assert!(graph.add_dep(crate3, SmolStr::new("crate1"), crate1).is_err());
302+
assert!(graph.add_dep(crate1, CrateName::new("crate2").unwrap(), crate2).is_ok());
303+
assert!(graph.add_dep(crate2, CrateName::new("crate3").unwrap(), crate3).is_ok());
304+
assert!(graph.add_dep(crate3, CrateName::new("crate1").unwrap(), crate1).is_err());
285305
}
286306

287307
#[test]
@@ -293,7 +313,23 @@ mod tests {
293313
graph.add_crate_root(FileId(2u32), Edition2018, CfgOptions::default(), Env::default());
294314
let crate3 =
295315
graph.add_crate_root(FileId(3u32), Edition2018, CfgOptions::default(), Env::default());
296-
assert!(graph.add_dep(crate1, SmolStr::new("crate2"), crate2).is_ok());
297-
assert!(graph.add_dep(crate2, SmolStr::new("crate3"), crate3).is_ok());
316+
assert!(graph.add_dep(crate1, CrateName::new("crate2").unwrap(), crate2).is_ok());
317+
assert!(graph.add_dep(crate2, CrateName::new("crate3").unwrap(), crate3).is_ok());
318+
}
319+
320+
#[test]
321+
fn dashes_are_normalized() {
322+
let mut graph = CrateGraph::default();
323+
let crate1 =
324+
graph.add_crate_root(FileId(1u32), Edition2018, CfgOptions::default(), Env::default());
325+
let crate2 =
326+
graph.add_crate_root(FileId(2u32), Edition2018, CfgOptions::default(), Env::default());
327+
assert!(graph
328+
.add_dep(crate1, CrateName::normalize_dashes("crate-name-with-dashes"), crate2)
329+
.is_ok());
330+
assert_eq!(
331+
graph.dependencies(crate1).collect::<Vec<_>>(),
332+
vec![&Dependency { crate_id: crate2, name: "crate_name_with_dashes".into() }]
333+
);
298334
}
299335
}

‎crates/ra_db/src/lib.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ use ra_syntax::{ast, Parse, SourceFile, TextRange, TextUnit};
1010

1111
pub use crate::{
1212
cancellation::Canceled,
13-
input::{CrateGraph, CrateId, Dependency, Edition, Env, FileId, SourceRoot, SourceRootId},
13+
input::{
14+
CrateGraph, CrateId, CrateName, Dependency, Edition, Env, FileId, SourceRoot, SourceRootId,
15+
},
1416
};
1517
pub use relative_path::{RelativePath, RelativePathBuf};
1618
pub use salsa;

‎crates/ra_ide/src/mock_analysis.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
use std::sync::Arc;
44

55
use ra_cfg::CfgOptions;
6-
use ra_db::{Env, RelativePathBuf};
6+
use ra_db::{CrateName, Env, RelativePathBuf};
77
use test_utils::{extract_offset, extract_range, parse_fixture, CURSOR_MARKER};
88

99
use crate::{
@@ -107,7 +107,9 @@ impl MockAnalysis {
107107
crate_graph.add_crate_root(file_id, Edition2018, cfg_options, Env::default());
108108
let crate_name = path.parent().unwrap().file_name().unwrap();
109109
if let Some(root_crate) = root_crate {
110-
crate_graph.add_dep(root_crate, crate_name.into(), other_crate).unwrap();
110+
crate_graph
111+
.add_dep(root_crate, CrateName::new(crate_name).unwrap(), other_crate)
112+
.unwrap();
111113
}
112114
}
113115
change.add_file(source_root, file_id, path, Arc::new(contents));

‎crates/ra_project_model/src/lib.rs

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use std::{
1313
};
1414

1515
use ra_cfg::CfgOptions;
16-
use ra_db::{CrateGraph, CrateId, Edition, Env, FileId};
16+
use ra_db::{CrateGraph, CrateId, CrateName, Edition, Env, FileId};
1717
use rustc_hash::FxHashMap;
1818
use serde_json::from_reader;
1919

@@ -177,7 +177,9 @@ impl ProjectWorkspace {
177177
if let (Some(&from), Some(&to)) =
178178
(crates.get(&from_crate_id), crates.get(&to_crate_id))
179179
{
180-
if let Err(_) = crate_graph.add_dep(from, dep.name.clone().into(), to) {
180+
if let Err(_) =
181+
crate_graph.add_dep(from, CrateName::new(&dep.name).unwrap(), to)
182+
{
181183
log::error!(
182184
"cyclic dependency {:?} -> {:?}",
183185
from_crate_id,
@@ -215,7 +217,9 @@ impl ProjectWorkspace {
215217
if let (Some(&from), Some(&to)) =
216218
(sysroot_crates.get(&from), sysroot_crates.get(&to))
217219
{
218-
if let Err(_) = crate_graph.add_dep(from, name.into(), to) {
220+
if let Err(_) =
221+
crate_graph.add_dep(from, CrateName::new(name).unwrap(), to)
222+
{
219223
log::error!("cyclic dependency between sysroot crates")
220224
}
221225
}
@@ -257,7 +261,7 @@ impl ProjectWorkspace {
257261
if let Some(proc_macro) = libproc_macro {
258262
if let Err(_) = crate_graph.add_dep(
259263
crate_id,
260-
"proc_macro".into(),
264+
CrateName::new("proc_macro").unwrap(),
261265
proc_macro,
262266
) {
263267
log::error!(
@@ -276,9 +280,14 @@ impl ProjectWorkspace {
276280
for &from in pkg_crates.get(&pkg).into_iter().flatten() {
277281
if let Some(to) = lib_tgt {
278282
if to != from {
279-
if let Err(_) =
280-
crate_graph.add_dep(from, pkg.name(&cargo).into(), to)
281-
{
283+
if let Err(_) = crate_graph.add_dep(
284+
from,
285+
// For root projects with dashes in their name,
286+
// cargo metadata does not do any normalization,
287+
// so we do it ourselves currently
288+
CrateName::normalize_dashes(pkg.name(&cargo)),
289+
to,
290+
) {
282291
log::error!(
283292
"cyclic dependency between targets of {}",
284293
pkg.name(&cargo)
@@ -289,17 +298,23 @@ impl ProjectWorkspace {
289298
// core is added as a dependency before std in order to
290299
// mimic rustcs dependency order
291300
if let Some(core) = libcore {
292-
if let Err(_) = crate_graph.add_dep(from, "core".into(), core) {
301+
if let Err(_) =
302+
crate_graph.add_dep(from, CrateName::new("core").unwrap(), core)
303+
{
293304
log::error!("cyclic dependency on core for {}", pkg.name(&cargo))
294305
}
295306
}
296307
if let Some(alloc) = liballoc {
297-
if let Err(_) = crate_graph.add_dep(from, "alloc".into(), alloc) {
308+
if let Err(_) =
309+
crate_graph.add_dep(from, CrateName::new("alloc").unwrap(), alloc)
310+
{
298311
log::error!("cyclic dependency on alloc for {}", pkg.name(&cargo))
299312
}
300313
}
301314
if let Some(std) = libstd {
302-
if let Err(_) = crate_graph.add_dep(from, "std".into(), std) {
315+
if let Err(_) =
316+
crate_graph.add_dep(from, CrateName::new("std").unwrap(), std)
317+
{
303318
log::error!("cyclic dependency on std for {}", pkg.name(&cargo))
304319
}
305320
}
@@ -312,9 +327,11 @@ impl ProjectWorkspace {
312327
for dep in pkg.dependencies(&cargo) {
313328
if let Some(&to) = pkg_to_lib_crate.get(&dep.pkg) {
314329
for &from in pkg_crates.get(&pkg).into_iter().flatten() {
315-
if let Err(_) =
316-
crate_graph.add_dep(from, dep.name.clone().into(), to)
317-
{
330+
if let Err(_) = crate_graph.add_dep(
331+
from,
332+
CrateName::new(&dep.name).unwrap(),
333+
to,
334+
) {
318335
log::error!(
319336
"cyclic dependency {} -> {}",
320337
pkg.name(&cargo),

0 commit comments

Comments
 (0)