Skip to content

Commit a9896a6

Browse files
committed
cargo-rail: adding the 'rail-substrate' lint package to improve the codebase's safety in production
1 parent 893f8e4 commit a9896a6

File tree

12 files changed

+85
-48
lines changed

12 files changed

+85
-48
lines changed

Cargo.lock

Lines changed: 12 additions & 12 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ exclude = [
2929

3030
[dependencies]
3131
clap = { version = "4.5.53", features = ["derive", "cargo"] }
32-
clap_complete = "4.5"
32+
clap_complete = "4.5.62"
3333
cargo_metadata = "0.23.1"
3434
petgraph = { version = "0.8.3", default-features = false }
3535
toml_edit = { version = "0.23.9", features = ["serde"] }
@@ -45,6 +45,25 @@ glob = "0.3.3"
4545
anyhow = "1.0.100"
4646
tempfile = "3.23.0"
4747

48+
[workspace.lints.rust]
49+
unsafe_code = "forbid"
50+
dangling_pointers_from_locals = "deny"
51+
integer_to_ptr_transmutes = "deny"
52+
53+
[workspace.lints.clippy]
54+
correctness = { level = "deny", priority = -1 }
55+
suspicious = { level = "deny", priority = -1 }
56+
perf = { level = "deny", priority = -1 }
57+
58+
std_instead_of_core = "warn"
59+
std_instead_of_alloc = "warn"
60+
unwrap_used = "allow"
61+
expect_used = "allow"
62+
63+
[workspace.lints.rustdoc]
64+
broken_intra_doc_links = "deny"
65+
private_intra_doc_links = "warn"
66+
4867
[[test]]
4968
name = "integration"
5069
path = "tests/integration/mod.rs"

src/cargo/manifest_writer.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,11 @@ impl ManifestWriter {
5858
}
5959

6060
// Second pass: write target-specific dependencies
61-
for dep in deps.iter().filter(|d| d.target.is_some()) {
61+
for dep in deps.iter() {
62+
let Some(target) = dep.target.as_ref() else {
63+
continue;
64+
};
6265
let entry = manifest_ops::build_dep_entry(dep);
63-
let target = dep.target.as_ref().unwrap();
6466
manifest_ops::insert_target_dependency(&mut doc, target, "dependencies", &dep.name, entry)
6567
.context("Failed to insert target dependency")?;
6668
}

src/cargo/unify_analyzer.rs

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -200,23 +200,21 @@ impl UnifyAnalyzer {
200200

201201
// Always use highest version (cargo's resolver already picks highest compatible)
202202
// When targets resolve to different versions, we unify to highest
203-
let version = if unique_versions.len() > 1 {
204-
// Multiple versions found across targets - use highest
205-
// This is the "silent win" - we unify duplicates automatically
206-
let selected = unique_versions.iter().max().unwrap();
207-
208-
// Track the cleanup for reporting
209-
let mut versions_found: Vec<_> = unique_versions.iter().map(|v| v.to_string()).collect();
210-
versions_found.sort();
211-
duplicates_cleaned.push(DuplicateCleanup {
212-
dep_name: dep_key.name.to_string(),
213-
versions_found,
214-
selected_version: selected.to_string(),
215-
});
216-
217-
selected
218-
} else {
219-
unique_versions.iter().next().unwrap()
203+
let version = match unique_versions.iter().max() {
204+
Some(max) if unique_versions.len() > 1 => {
205+
// Multiple versions found across targets - use highest
206+
// This is the "silent win" - we unify duplicates automatically
207+
let mut versions_found: Vec<_> = unique_versions.iter().map(|v| v.to_string()).collect();
208+
versions_found.sort();
209+
duplicates_cleaned.push(DuplicateCleanup {
210+
dep_name: dep_key.name.to_string(),
211+
versions_found,
212+
selected_version: max.to_string(),
213+
});
214+
max
215+
}
216+
Some(version) => version,
217+
None => continue, // Empty set - shouldn't happen given versions check above
220218
};
221219

222220
// Construct version requirement - preserve exact pins if configured
@@ -227,9 +225,15 @@ impl UnifyAnalyzer {
227225
.find_map(|u| u.declared_version.as_ref().filter(|v| is_exact_pin(v)))
228226
.cloned()
229227
.unwrap_or_else(|| format!("={}", version));
230-
VersionReq::parse(&exact_version).unwrap_or_else(|_| VersionReq::parse(&format!("^{}", version)).unwrap())
228+
// Try exact version first, fall back to caret format, then ultimate fallback
229+
VersionReq::parse(&exact_version)
230+
.or_else(|_| VersionReq::parse(&format!("^{}", version)))
231+
.unwrap_or_else(|_| VersionReq::default())
231232
} else {
232-
VersionReq::parse(&format!("^{}", version)).unwrap_or_else(|_| VersionReq::parse(&version.to_string()).unwrap())
233+
// Try caret format first, fall back to plain version, then ultimate fallback
234+
VersionReq::parse(&format!("^{}", version))
235+
.or_else(|_| VersionReq::parse(&version.to_string()))
236+
.unwrap_or_else(|_| VersionReq::default())
233237
};
234238

235239
// Check for version mismatches with existing workspace.dependencies

src/commands/config.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -796,8 +796,8 @@ fn sync_targets(editor: &mut TomlEditor, workspace_root: &Path) -> RailResult<Op
796796
.map_err(|e| RailError::message(format!("Failed to format targets: {}", e)))?;
797797

798798
// Extract the value and insert it
799-
if let Some(targets_item) = parsed.get("targets") {
800-
editor.doc_mut()["targets"] = Item::Value(targets_item.as_value().unwrap().clone());
799+
if let Some(value) = parsed.get("targets").and_then(|item| item.as_value()) {
800+
editor.doc_mut()["targets"] = Item::Value(value.clone());
801801
}
802802

803803
Ok(Some(TargetSyncResult {

src/commands/release.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,10 @@ pub fn run_release_init(ctx: &WorkspaceContext, crates: Option<Vec<String>>, che
428428

429429
new_crates.push(pkg.name.clone());
430430

431-
let crate_dir = pkg.manifest_path.parent().expect("manifest has parent");
431+
let Some(crate_dir) = pkg.manifest_path.parent() else {
432+
// manifest_path always has a parent directory - skip if somehow malformed
433+
continue;
434+
};
432435
let changelog_path = crate::utils::detect_crate_changelog(crate_dir);
433436

434437
let crate_config = config.crates.entry(pkg.name.to_string()).or_default();

src/commands/split.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,10 @@ fn detect_workspace_splits(
276276
}
277277

278278
// Get relative path from workspace root to crate directory
279-
let crate_dir = pkg.manifest_path.parent().expect("manifest has parent");
279+
let Some(crate_dir) = pkg.manifest_path.parent() else {
280+
// manifest_path always has a parent directory - skip if somehow malformed
281+
continue;
282+
};
280283
// Detect per-crate CHANGELOG file
281284
let changelog_path = crate::utils::detect_crate_changelog(crate_dir);
282285
let rel_path = match crate_dir.strip_prefix(workspace_root) {

src/graph/core.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -276,8 +276,8 @@ impl WorkspaceGraph {
276276
if neighbor_node.is_workspace_member
277277
&& let Some(&to_subgraph_idx) = name_to_subgraph_idx.get(&neighbor_node.name)
278278
&& let Some(edge) = self.graph.find_edge(from_graph_idx, neighbor_graph_idx)
279+
&& let Some(&kind) = self.graph.edge_weight(edge)
279280
{
280-
let kind = *self.graph.edge_weight(edge).unwrap();
281281
// Skip dev-dependencies - they don't affect publish order and can have cycles
282282
// Also skip self-references (crate depending on itself for test features)
283283
if kind != DependencyKind::Development && from_name != &neighbor_node.name {
@@ -321,10 +321,8 @@ impl WorkspaceGraph {
321321
// Normalize to workspace-relative path
322322
let relative_path = self.to_workspace_relative(file_path)?;
323323

324-
let cache = self
325-
.path_cache
326-
.read()
327-
.expect("RwLock poisoned: another thread panicked while holding the lock");
324+
// If the lock is poisoned (another thread panicked), return None gracefully
325+
let cache = self.path_cache.read().ok()?;
328326
let cache_ref = cache.as_ref()?;
329327

330328
// Walk up directory tree looking for a crate root
@@ -433,7 +431,11 @@ impl WorkspaceGraph {
433431
}
434432
}
435433

436-
*self.path_cache.write().unwrap() = Some(cache);
434+
// If the lock is poisoned (another thread panicked), skip cache update
435+
// The cache will be rebuilt on next access attempt
436+
if let Ok(mut guard) = self.path_cache.write() {
437+
*guard = Some(cache);
438+
}
437439
}
438440
}
439441

src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
33
#![warn(missing_docs)]
44
#![cfg_attr(docsrs, feature(doc_cfg))]
5+
#![cfg_attr(not(test), deny(clippy::unwrap_used))]
6+
#![cfg_attr(not(test), deny(clippy::expect_used))]
57

68
/// Backup and restore for undo operations.
79
pub mod backup;

src/main.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
//! Entry point for cargo-rail
22
//!
33
//! This is intentionally thin - all logic lives in the library.
4+
#![cfg_attr(not(test), deny(clippy::unwrap_used))]
5+
#![cfg_attr(not(test), deny(clippy::expect_used))]
46

57
use cargo_rail::commands::cli::{ConfigCommand, UnifyCommand};
68
use cargo_rail::commands::{self, CargoCli, Commands, StrictnessMode};

0 commit comments

Comments
 (0)