Skip to content

Commit

Permalink
Add nostd lint (#93)
Browse files Browse the repository at this point in the history
* Add nostd check

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* add nostd checks

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Add minimal test

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Bump to 1.4.0

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
  • Loading branch information
ggwpez committed Apr 9, 2024
1 parent 9f047b8 commit 4158cc6
Show file tree
Hide file tree
Showing 11 changed files with 339 additions and 32 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "zepter"
version = "1.3.1"
version = "1.4.0"
edition = "2021"
authors = [ "Oliver Tale-Yazdi" ]
description = "Analyze, Fix and Format features in your Rust workspace."
Expand Down
44 changes: 32 additions & 12 deletions src/autofix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use std::{
collections::BTreeMap as Map,
path::{Path, PathBuf},
};
use toml_edit::{table, value, Array, Document, Formatted, InlineTable, Item, Table, Value};
use toml_edit::{table, value, Array, DocumentMut, Formatted, InlineTable, Item, Table, Value};

#[derive(Debug, clap::Parser)]
#[cfg_attr(feature = "testing", derive(Default))]
Expand All @@ -21,20 +21,20 @@ pub struct AutoFixerArgs {

pub struct AutoFixer {
pub manifest: Option<PathBuf>,
doc: Option<Document>,
doc: Option<DocumentMut>,
raw: String,
}

impl AutoFixer {
pub fn from_manifest<P: AsRef<Path>>(manifest: P) -> Result<Self, String> {
let raw = std::fs::read_to_string(&manifest)
.map_err(|e| format!("Failed to read manifest: {e}"))?;
let doc = raw.parse::<Document>().map_err(|e| format!("Failed to parse manifest: {e}"))?;
let doc = raw.parse::<DocumentMut>().map_err(|e| format!("Failed to parse manifest: {e}"))?;
Ok(Self { raw, manifest: Some(manifest.as_ref().to_path_buf()), doc: Some(doc) })
}

pub fn from_raw(raw: &str) -> Result<Self, String> {
let doc = raw.parse::<Document>().map_err(|e| format!("Failed to parse manifest: {e}"))?;
let doc = raw.parse::<DocumentMut>().map_err(|e| format!("Failed to parse manifest: {e}"))?;
Ok(Self { raw: raw.into(), manifest: None, doc: Some(doc) })
}

Expand Down Expand Up @@ -237,7 +237,7 @@ impl AutoFixer {
fn get_all_features(&self) -> Vec<String> {
let mut found = Vec::new();

let doc: &Document = self.doc.as_ref().unwrap();
let doc: &DocumentMut = self.doc.as_ref().unwrap();
if !doc.contains_table("features") {
return found
}
Expand All @@ -251,7 +251,7 @@ impl AutoFixer {
}

fn get_feature(&self, name: &str) -> Option<&Array> {
let doc: &Document = self.doc.as_ref().unwrap();
let doc: &DocumentMut = self.doc.as_ref().unwrap();
if !doc.contains_table("features") {
return None
}
Expand All @@ -265,7 +265,7 @@ impl AutoFixer {
}

pub(crate) fn get_feature_mut(&mut self, name: &str) -> Result<&mut Array, ()> {
let doc: &mut Document = self.doc.as_mut().unwrap();
let doc: &mut DocumentMut = self.doc.as_mut().unwrap();
if !doc.contains_table("features") {
return Err(())
}
Expand Down Expand Up @@ -358,7 +358,7 @@ impl AutoFixer {
}

pub fn add_feature(&mut self, feature: &str) -> Result<(), String> {
let doc: &mut Document = self.doc.as_mut().unwrap();
let doc: &mut DocumentMut = self.doc.as_mut().unwrap();

if !doc.contains_table("features") {
doc.as_table_mut().insert("features", table());
Expand All @@ -375,7 +375,7 @@ impl AutoFixer {

/// Add something to a feature. Creates that feature if it does not exist.
pub fn add_to_feature(&mut self, feature: &str, v: &str) -> Result<(), String> {
let doc: &mut Document = self.doc.as_mut().unwrap();
let doc: &mut DocumentMut = self.doc.as_mut().unwrap();

if !doc.contains_table("features") {
doc.as_table_mut().insert("features", table());
Expand Down Expand Up @@ -454,7 +454,7 @@ impl AutoFixer {
default_feats: Option<bool>,
) -> Result<(), String> {
let kind = crate::kind_to_str(kind);
let doc: &mut Document = self.doc.as_mut().unwrap();
let doc: &mut DocumentMut = self.doc.as_mut().unwrap();

if !doc.contains_table(kind) {
return Ok(())
Expand Down Expand Up @@ -520,7 +520,7 @@ impl AutoFixer {
) -> Result<(), String> {
// The carrot is implicit in cargo.
let version_str = dep_version.to_string().trim_start_matches('^').to_string();
let doc: &mut Document = self.doc.as_mut().unwrap();
let doc: &mut DocumentMut = self.doc.as_mut().unwrap();

if !doc.contains_table("workspace") {
return Err("No workspace entry found".into())
Expand Down Expand Up @@ -578,7 +578,7 @@ impl AutoFixer {
}

pub fn remove_feature(&mut self, name: &str) {
let doc: &mut Document = self.doc.as_mut().unwrap();
let doc: &mut DocumentMut = self.doc.as_mut().unwrap();

if !doc.contains_table("features") {
return
Expand All @@ -601,6 +601,26 @@ impl AutoFixer {
}
}

pub fn disable_default_features(&mut self, dep: &str) -> Result<(), String> {
let doc: &mut DocumentMut = self.doc.as_mut().unwrap();

if !doc.contains_table("dependencies") {
return Err("No dependencies entry found".into())
}

let deps = doc["dependencies"].as_table_mut().unwrap();
let Some(dep) = deps.get_mut(dep) else {
return Err(format!("Dependency '{}' not found", dep))
};

if let Some(dep) = dep.as_inline_table_mut() {
dep.insert("default-features", Value::Boolean(Formatted::new(false)));
Ok(())
} else {
Err(format!("Dependency '{}' is not an inline table", dep))
}
}

pub fn modified(&self) -> bool {
self.doc.as_ref().unwrap().to_string() != self.raw
}
Expand Down
6 changes: 6 additions & 0 deletions src/cmd/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@

//! Lint your feature usage by analyzing crate metadata.

pub mod nostd;
pub use nostd::*;

use crate::{
autofix::*,
cmd::{parse_key_val, resolve_dep, RenamedPackage},
Expand Down Expand Up @@ -43,6 +46,8 @@ pub enum SubCommand {
/// A specific feature is only implied by a specific set of other features.
OnlyEnables(OnlyEnablesCmd),
WhyEnabled(WhyEnabledCmd),
/// Check the crates for sane no-std feature configuration.
NoStd(NoStdCmd),
}

#[derive(Debug, clap::Parser)]
Expand Down Expand Up @@ -272,6 +277,7 @@ impl LintCmd {
cmd.run(global);
Ok(())
},
SubCommand::NoStd(cmd) => cmd.run(global),
}
}
}
Expand Down
137 changes: 137 additions & 0 deletions src/cmd/lint/nostd.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
// SPDX-License-Identifier: GPL-3.0-only
// SPDX-FileCopyrightText: Oliver Tale-Yazdi <oliver@tasty.limo>

use std::collections::BTreeMap;
use crate::cmd::{lint::AutoFixer, CargoArgs};
use crate::cmd::GlobalArgs;
use cargo_metadata::{DependencyKind, Package};
use crate::cmd::resolve_dep;
use crate::grammar::plural;
use std::collections::btree_map::Entry;
use std::fs::canonicalize;
use crate::log;

#[derive(Debug, clap::Parser)]
pub struct NoStdCmd {
#[clap(subcommand)]
sub: NoStdSubCmd,
}

#[derive(Debug, clap::Subcommand)]
pub enum NoStdSubCmd {
/// Default features of no-std dependencies are disabled if the crate itself supports no-std.
#[clap(name = "default-features-of-nostd-dependencies-disabled")]
DefaultFeaturesDisabled(DefaultFeaturesDisabledCmd),
}

#[derive(Debug, clap::Parser)]
pub struct DefaultFeaturesDisabledCmd {
#[allow(missing_docs)]
#[clap(flatten)]
cargo_args: CargoArgs,

/// Whether to fix the issues.
#[clap(long, short)]
fix: bool,
}

impl NoStdCmd {
pub(crate) fn run(&self, global: &GlobalArgs) -> Result<(), String> {
match &self.sub {
NoStdSubCmd::DefaultFeaturesDisabled(cmd) => cmd.run(global),
}
}
}

impl DefaultFeaturesDisabledCmd {
pub(crate) fn run(&self, _: &GlobalArgs) -> Result<(), String> {
let meta = self.cargo_args.clone().with_workspace(true).load_metadata()?;
let pkgs = &meta.packages;
let mut cache = BTreeMap::new();
let mut autofixer = BTreeMap::new();
let mut issues = 0;
// Dir that we are allowed to write to.
let allowed_dir = canonicalize(meta.workspace_root.as_std_path()).unwrap();

for lhs in pkgs.iter() {
// check if lhs supports no-std builds
if !Self::supports_nostd(lhs, &mut cache)? {
continue;
}

for dep in lhs.dependencies.iter() {
if dep.kind != DependencyKind::Normal {
continue;
}

let Some(rhs) = resolve_dep(lhs, dep, &meta) else {
continue
};

if !Self::supports_nostd(&rhs.pkg, &mut cache)? {
continue;
}

if !dep.uses_default_features {
continue;
}

println!("Default features not disabled for dependency: {} -> {}", lhs.name, rhs.pkg.name);

let fixer = match autofixer.entry(lhs.manifest_path.clone()) {
Entry::Occupied(e) => e.into_mut(),
Entry::Vacant(e) => {
let krate_path = canonicalize(lhs.manifest_path.clone().into_std_path_buf()).unwrap();

if !krate_path.starts_with(&allowed_dir) {
return Err(format!("Cannot write to path: {}", krate_path.display()))
}
e.insert(AutoFixer::from_manifest(&lhs.manifest_path)?)
},
};

fixer.disable_default_features(&rhs.name())?;
issues += 1;
}
}

let s = plural(autofixer.len());
print!("Found {} issue{} in {} crate{s} ", issues, plural(issues), autofixer.len());
if self.fix {
for (_, fixer) in autofixer.iter_mut() {
fixer.save()?;
}
println!("and fixed all of them.");
Ok(())
} else {
println!("and fixed none. Re-run with --fix to apply fixes.");
Err(format!("Several issues were not fixed."))
}
}

fn supports_nostd(krate: &Package, cache: &mut BTreeMap<String, bool>) -> Result<bool, String> {
log::debug!("Checking if crate supports no-std: {}", krate.name);
if let Some(res) = cache.get(krate.manifest_path.as_str()) {
return Ok(*res)
}

// try to find the lib.rs
let krate_root = krate.manifest_path.parent().ok_or_else(|| format!("Could not find parent of manifest: {}", krate.manifest_path))?;
let lib_rs = krate_root.join("src/lib.rs");

if !lib_rs.exists() {
return Ok(false)
}
let content = std::fs::read_to_string(&lib_rs).map_err(|e| format!("Could not read lib.rs: {}", e))?;

let ret = if content.contains("#![cfg_attr(not(feature = \"std\"), no_std)]") || content.contains("#![no_std]") {
log::debug!("Crate supports no-std: {} (path={})", krate.name, krate.manifest_path);
true
} else {
false
};

cache.insert(krate.manifest_path.as_str().into(), ret);
Ok(ret)
}
}
Loading

0 comments on commit 4158cc6

Please sign in to comment.