Skip to content

Commit 3608b72

Browse files
committed
more review changes
fix logic op and convert path to abs dir
1 parent 1984565 commit 3608b72

File tree

2 files changed

+41
-8
lines changed

2 files changed

+41
-8
lines changed

cpp-linter/src/clang_tools/mod.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ impl ClangTool {
6262
match version {
6363
RequestedVersion::Path(path_buf) => {
6464
which_in(name, Some(path_buf), current_dir().unwrap())
65-
.map_err(|_| anyhow!("Could not find {name} by path"))
65+
.map_err(|_| anyhow!("Could not find {self} by path"))
6666
}
6767
// Thus, we should use whatever is installed and added to $PATH.
6868
RequestedVersion::SystemDefault | RequestedVersion::NoValue => {
@@ -77,7 +77,7 @@ impl ClangTool {
7777
let mut it = req.comparators.iter();
7878
let mut highest_major = it.next().map(|v| v.major).unwrap_or_default() + 1;
7979
for n in it {
80-
if n.major < highest_major {
80+
if n.major > highest_major {
8181
// +1 because we aren't checking the comparator's operator here.
8282
highest_major = n.major + 1;
8383
}
@@ -95,7 +95,7 @@ impl ClangTool {
9595

9696
// now we're ready to search for the binary exe with the major version suffixed.
9797
for major in majors {
98-
if let Ok(cmd) = which(format!("{name}-{major}")) {
98+
if let Ok(cmd) = which(format!("{self}-{major}")) {
9999
return Ok(cmd);
100100
}
101101
}
@@ -109,7 +109,7 @@ impl ClangTool {
109109
// On Unix systems, this line is not likely reached. Typically, installing clang
110110
// will produce a symlink to the executable with the major version appended to the
111111
// name.
112-
which(name).map_err(|_| anyhow!("Could not find {name} by version"))
112+
which(name).map_err(|_| anyhow!("Could not find {self} by version"))
113113
}
114114
}
115115
}

cpp-linter/src/cli/structs.rs

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@ use clap::{builder::PossibleValue, ValueEnum};
55
use semver::VersionReq;
66

77
use super::Cli;
8-
use crate::{clang_tools::clang_tidy::CompilationUnit, common_fs::FileFilter};
8+
use crate::{
9+
clang_tools::clang_tidy::CompilationUnit,
10+
common_fs::{normalize_path, FileFilter},
11+
};
912

1013
#[derive(Debug, Clone, PartialEq, Eq, Default)]
1114
pub enum RequestedVersion {
@@ -48,9 +51,20 @@ impl FromStr for RequestedVersion {
4851
input
4952
));
5053
}
51-
let path = path
52-
.canonicalize()
53-
.map_err(|e| anyhow!("Failed to canonicalize path '{input}': {e:?}"))?;
54+
let path = if !path.is_dir() {
55+
path.parent()
56+
.ok_or(anyhow!(
57+
"Unknown parent directory of the given file path for `--version`: {}",
58+
input
59+
))?
60+
.to_path_buf()
61+
} else {
62+
path
63+
};
64+
let path = match path.canonicalize() {
65+
Ok(p) => Ok(normalize_path(&p)),
66+
Err(e) => Err(anyhow!("Failed to canonicalize path '{input}': {e:?}")),
67+
}?;
5468
Ok(Self::Path(path))
5569
}
5670
}
@@ -292,6 +306,10 @@ impl Default for FeedbackInput {
292306
mod test {
293307
// use crate::cli::get_arg_parser;
294308

309+
use std::{path::PathBuf, str::FromStr};
310+
311+
use crate::{cli::RequestedVersion, common_fs::normalize_path};
312+
295313
use super::{Cli, LinesChangedOnly, ThreadComments};
296314
use clap::{Parser, ValueEnum};
297315

@@ -332,4 +350,19 @@ mod test {
332350
ThreadComments::Off
333351
);
334352
}
353+
354+
#[test]
355+
fn validate_version_path() {
356+
let this_path_str = "src/cli/structs.rs";
357+
let this_path = PathBuf::from(this_path_str);
358+
let this_canonical = this_path.canonicalize().unwrap();
359+
let parent = this_canonical.parent().unwrap();
360+
let expected = normalize_path(parent);
361+
let req_ver = RequestedVersion::from_str(this_path_str).unwrap();
362+
if let RequestedVersion::Path(parsed) = req_ver {
363+
assert_eq!(&parsed, &expected);
364+
}
365+
366+
assert!(RequestedVersion::from_str("file.rs").is_err());
367+
}
335368
}

0 commit comments

Comments
 (0)