|
1 | 1 | use crate::core::{Edition, Feature, Features, Manifest, MaybePackage, Package}; |
2 | 2 | use crate::{CargoResult, GlobalContext}; |
| 3 | + |
3 | 4 | use annotate_snippets::{AnnotationKind, Group, Level, Patch, Snippet}; |
4 | | -use cargo_util_schemas::manifest::{ProfilePackageSpec, TomlLintLevel, TomlToolLints}; |
| 5 | +use cargo_util_schemas::manifest::ProfilePackageSpec; |
| 6 | +use cargo_util_schemas::manifest::TomlLintLevel; |
| 7 | +use cargo_util_schemas::manifest::TomlToolLints; |
5 | 8 | use pathdiff::diff_paths; |
| 9 | + |
6 | 10 | use std::borrow::Cow; |
| 11 | +use std::collections::HashMap; |
7 | 12 | use std::fmt::Display; |
8 | 13 | use std::ops::Range; |
9 | 14 | use std::path::Path; |
10 | 15 |
|
11 | 16 | const LINT_GROUPS: &[LintGroup] = &[TEST_DUMMY_UNSTABLE]; |
12 | | -pub const LINTS: &[Lint] = &[BLANKET_HINT_MOSTLY_UNUSED, IM_A_TEAPOT, UNKNOWN_LINTS]; |
| 17 | + |
| 18 | +pub const LINTS: &[Lint] = &[ |
| 19 | + BLANKET_HINT_MOSTLY_UNUSED, |
| 20 | + IMPRECISE_VERSION_REQUIREMENTS, |
| 21 | + IM_A_TEAPOT, |
| 22 | + UNKNOWN_LINTS, |
| 23 | +]; |
| 24 | + |
| 25 | +enum SpanOrigin { |
| 26 | + Specified(core::ops::Range<usize>), |
| 27 | + Inherited(core::ops::Range<usize>), |
| 28 | +} |
13 | 29 |
|
14 | 30 | pub fn analyze_cargo_lints_table( |
15 | 31 | pkg: &Package, |
@@ -743,6 +759,189 @@ fn output_unknown_lints( |
743 | 759 | Ok(()) |
744 | 760 | } |
745 | 761 |
|
| 762 | +const IMPRECISE_VERSION_REQUIREMENTS: Lint = Lint { |
| 763 | + name: "imprecise_version_requirements", |
| 764 | + desc: "dependency version requirement lacks full precision", |
| 765 | + groups: &[], |
| 766 | + default_level: LintLevel::Allow, |
| 767 | + edition_lint_opts: None, |
| 768 | + feature_gate: None, |
| 769 | + docs: Some( |
| 770 | + r#" |
| 771 | +### What it does |
| 772 | +
|
| 773 | +Checks for dependency version requirements that lack full `major.minor.patch` precision, |
| 774 | +such as `serde = "1"` or `serde = "1.0"`. |
| 775 | +
|
| 776 | +### Why it is bad |
| 777 | +
|
| 778 | +Imprecise version requirements can be misleading about the actual minimum supported version. |
| 779 | +For example, |
| 780 | +`serde = "1"` suggests that any version from `1.0.0` onwards is acceptable, |
| 781 | +but if your code actually requires features from `1.0.219`, |
| 782 | +the imprecise requirement gives a false impression about compatibility. |
| 783 | +
|
| 784 | +Specifying the full version helps with: |
| 785 | +
|
| 786 | +- Accurate minimum version documentation |
| 787 | +- Better compatibility with `-Z minimal-versions` |
| 788 | +- Clearer dependency constraints for consumers |
| 789 | +
|
| 790 | +### Drawbacks |
| 791 | +
|
| 792 | +Even with fully specified versions, |
| 793 | +the minimum bound might still be incorrect if untested. |
| 794 | +This lint helps improve precision but doesn't guarantee correctness. |
| 795 | +
|
| 796 | +### Example |
| 797 | +
|
| 798 | +```toml |
| 799 | +[dependencies] |
| 800 | +serde = "1" |
| 801 | +``` |
| 802 | +
|
| 803 | +Should be written as a full specific version: |
| 804 | +
|
| 805 | +```toml |
| 806 | +[dependencies] |
| 807 | +serde = "1.0.219" |
| 808 | +``` |
| 809 | +"#, |
| 810 | + ), |
| 811 | +}; |
| 812 | + |
| 813 | +pub fn imprecise_version_requirements( |
| 814 | + pkg: &Package, |
| 815 | + path: &Path, |
| 816 | + pkg_lints: &TomlToolLints, |
| 817 | + ws_contents: &str, |
| 818 | + ws_document: &toml::Spanned<toml::de::DeTable<'static>>, |
| 819 | + ws_path: &Path, |
| 820 | + error_count: &mut usize, |
| 821 | + gctx: &GlobalContext, |
| 822 | +) -> CargoResult<()> { |
| 823 | + let manifest = pkg.manifest(); |
| 824 | + let (lint_level, reason) = IMPRECISE_VERSION_REQUIREMENTS.level( |
| 825 | + pkg_lints, |
| 826 | + manifest.edition(), |
| 827 | + manifest.unstable_features(), |
| 828 | + ); |
| 829 | + |
| 830 | + if lint_level == LintLevel::Allow { |
| 831 | + return Ok(()); |
| 832 | + } |
| 833 | + |
| 834 | + let manifest_path = rel_cwd_manifest_path(path, gctx); |
| 835 | + |
| 836 | + let platform_map: HashMap<cargo_platform::Platform, String> = manifest |
| 837 | + .normalized_toml() |
| 838 | + .target |
| 839 | + .as_ref() |
| 840 | + .map(|map| { |
| 841 | + map.keys() |
| 842 | + .map(|k| (k.parse().expect("already parsed"), k.clone())) |
| 843 | + .collect() |
| 844 | + }) |
| 845 | + .unwrap_or_default(); |
| 846 | + |
| 847 | + for dep in manifest.dependencies().iter() { |
| 848 | + let crate::util::OptVersionReq::Req(req) = dep.version_req() else { |
| 849 | + continue; |
| 850 | + }; |
| 851 | + let [cmp] = req.comparators.as_slice() else { |
| 852 | + continue; |
| 853 | + }; |
| 854 | + if cmp.op != semver::Op::Caret { |
| 855 | + continue; |
| 856 | + } |
| 857 | + if cmp.minor.is_some() && cmp.patch.is_some() { |
| 858 | + continue; |
| 859 | + } |
| 860 | + |
| 861 | + // Only focus on single caret requirement that has only `major` or `major.minor` |
| 862 | + let name_in_toml = dep.name_in_toml().as_str(); |
| 863 | + |
| 864 | + let key_path = if let Some(cfg) = dep.platform().and_then(|p| platform_map.get(p)) { |
| 865 | + &["target", &cfg, dep.kind().kind_table(), name_in_toml][..] |
| 866 | + } else { |
| 867 | + &[dep.kind().kind_table(), name_in_toml][..] |
| 868 | + }; |
| 869 | + |
| 870 | + let Some((_key, value)) = get_key_value(manifest.document(), key_path) else { |
| 871 | + continue; |
| 872 | + }; |
| 873 | + |
| 874 | + let span = match value.as_ref() { |
| 875 | + toml::de::DeValue::String(_) => SpanOrigin::Specified(value.span()), |
| 876 | + toml::de::DeValue::Table(map) => { |
| 877 | + if let Some(v) = map.get("version").filter(|v| v.as_ref().is_str()) { |
| 878 | + SpanOrigin::Specified(v.span()) |
| 879 | + } else if let Some((k, v)) = map |
| 880 | + .get_key_value("workspace") |
| 881 | + .filter(|(_, v)| v.as_ref().is_bool()) |
| 882 | + { |
| 883 | + SpanOrigin::Inherited(k.span().start..v.span().end) |
| 884 | + } else { |
| 885 | + panic!("version must be specified or workspace-inherited"); |
| 886 | + } |
| 887 | + } |
| 888 | + _ => unreachable!("dependency must be string or table"), |
| 889 | + }; |
| 890 | + |
| 891 | + let level = lint_level.to_diagnostic_level(); |
| 892 | + let title = IMPRECISE_VERSION_REQUIREMENTS.desc; |
| 893 | + let emitted_source = IMPRECISE_VERSION_REQUIREMENTS.emitted_source(lint_level, reason); |
| 894 | + let report = match span { |
| 895 | + SpanOrigin::Specified(span) => &[Group::with_title(level.clone().primary_title(title)) |
| 896 | + .element( |
| 897 | + Snippet::source(manifest.contents()) |
| 898 | + .path(&manifest_path) |
| 899 | + .annotation(AnnotationKind::Primary.span(span)), |
| 900 | + ) |
| 901 | + .element(Level::NOTE.message(emitted_source))][..], |
| 902 | + SpanOrigin::Inherited(inherit_span) => { |
| 903 | + let key_path = &["workspace", "dependencies", name_in_toml]; |
| 904 | + let (_, value) = |
| 905 | + get_key_value(ws_document, key_path).expect("must have workspace dep"); |
| 906 | + let ws_span = match value.as_ref() { |
| 907 | + toml::de::DeValue::String(_) => value.span(), |
| 908 | + toml::de::DeValue::Table(map) => map |
| 909 | + .get("version") |
| 910 | + .filter(|v| v.as_ref().is_str()) |
| 911 | + .map(|v| v.span()) |
| 912 | + .expect("must have a version field"), |
| 913 | + _ => unreachable!("dependency must be string or table"), |
| 914 | + }; |
| 915 | + |
| 916 | + let ws_path = rel_cwd_manifest_path(ws_path, gctx); |
| 917 | + let second_title = format!("dependency `{name_in_toml}` was inherited"); |
| 918 | + |
| 919 | + &[ |
| 920 | + Group::with_title(level.clone().primary_title(title)).element( |
| 921 | + Snippet::source(ws_contents) |
| 922 | + .path(ws_path) |
| 923 | + .annotation(AnnotationKind::Primary.span(ws_span)), |
| 924 | + ), |
| 925 | + Group::with_title(Level::NOTE.secondary_title(second_title)) |
| 926 | + .element( |
| 927 | + Snippet::source(manifest.contents()) |
| 928 | + .path(&manifest_path) |
| 929 | + .annotation(AnnotationKind::Context.span(inherit_span)), |
| 930 | + ) |
| 931 | + .element(Level::NOTE.message(emitted_source)), |
| 932 | + ][..] |
| 933 | + } |
| 934 | + }; |
| 935 | + |
| 936 | + if lint_level.is_error() { |
| 937 | + *error_count += 1; |
| 938 | + } |
| 939 | + gctx.shell().print_report(report, lint_level.force())?; |
| 940 | + } |
| 941 | + |
| 942 | + Ok(()) |
| 943 | +} |
| 944 | + |
746 | 945 | #[cfg(test)] |
747 | 946 | mod tests { |
748 | 947 | use itertools::Itertools; |
|
0 commit comments