Skip to content

Commit 4057f8d

Browse files
authored
Enable clippy linting in repo (#1114)
Branched from #1113 Closes #1104 - On CI. Config taken from https://github.com/astral-sh/ruff/blob/56eb6b62936142c6ab2bdf0d7b864e32399e02a8/.github/workflows/ci.yaml#L243-L263 - In IDEs via rust-analyzer's `check` config. I'm a bit hesitant enabling it in rust-analyser because of this (from #1104 (comment)): One UX downside to be aware of: After each edits, clippy runs longer checks on the repo, which locks other gestures requiring compilation (running tests etc). <img width="584" height="142" alt="Image" src="https://github.com/user-attachments/assets/8f6246c3-95a3-4331-a5f2-5d69d6a7f532" /> But I guess we can override in our personal configs if we want a different workflow (check with clippy before sending PR).
2 parents ab33926 + d682930 commit 4057f8d

File tree

10 files changed

+50
-21
lines changed

10 files changed

+50
-21
lines changed

.github/workflows/check-rust.yml

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,29 @@ jobs:
2020
- name: "Check formatting"
2121
run: cargo +nightly fmt --all --check
2222

23+
clippy:
24+
name: "Check clippy"
25+
runs-on: ubuntu-latest
26+
timeout-minutes: 20
27+
steps:
28+
- uses: actions/checkout@v4
29+
30+
- uses: Swatinem/rust-cache@v2
31+
with:
32+
save-if: ${{ github.ref == 'refs/heads/main' }}
33+
34+
- name: "Install clippy"
35+
run: rustup component add clippy
36+
37+
- name: Toolchain versions
38+
run: |
39+
rustc -Vv
40+
cargo -V
41+
cargo clippy -V
42+
43+
- name: "Check clippy"
44+
run: cargo clippy --all-targets --all-features --locked -- -D warnings
45+
2346
dependencies:
2447
name: "Check only workspace dependencies"
2548
runs-on: ubuntu-latest

.vscode/settings.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
"rust-analyzer.imports.granularity.group": "item",
1414
"rust-analyzer.imports.granularity.enforce": true,
1515
"rust-analyzer.showUnlinkedFileNotification": false,
16+
"rust-analyzer.check.command": "clippy",
1617
"rust-analyzer.check.extraArgs": [
1718
"--target-dir", "${workspaceFolder}/target/check"
1819
],

.zed/settings.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
"lsp": {
33
"rust-analyzer": {
44
"initialization_options": {
5+
"check": {
6+
"command": "clippy"
7+
},
58
"rustfmt": {
69
"extraArgs": ["+nightly"]
710
}

AGENTS.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,12 @@ just test <test_name>
5757
just test -p ark
5858
```
5959

60+
### Running Clippy
61+
62+
```sh
63+
just clippy
64+
```
65+
6066
### Kernel and DAP Test Infrastructure
6167

6268
Integration tests for the kernel and DAP server live in `crates/ark/tests/` and use the test utilities from `crates/ark_test/`.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ members = [
1919
]
2020

2121
[workspace.package]
22-
rust-version = "1.89"
22+
rust-version = "1.94"
2323
edition = "2021"
2424
license = "MIT"
2525
authors = ["Posit Software, PBC"]

crates/amalthea/src/kernel_dirs.rs

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,23 +30,12 @@ pub fn jupyter_dir() -> Option<PathBuf> {
3030
#[cfg(not(target_os = "macos"))]
3131
fn jupyter_xdg_dir() -> Option<PathBuf> {
3232
// On Windows/Linux, the path is XDG_DATA_DIR/jupyter
33-
if let Some(path) = dirs::data_dir() {
34-
Some(path.join("jupyter"))
35-
} else {
36-
None
37-
}
33+
dirs::data_dir().map(|path| path.join("jupyter"))
3834
}
3935

4036
#[cfg(target_os = "macos")]
4137
fn jupyter_xdg_dir() -> Option<PathBuf> {
4238
// On MacOS, XDG_DATA_DIR is ~/Library/Application Support, but Jupyter
4339
// looks in ~/Library/Jupyter.
44-
if let Some(path) = dirs::data_dir() {
45-
if let Some(parent) = path.parent() {
46-
return Some(parent.join("Jupyter"));
47-
} else {
48-
return None;
49-
}
50-
}
51-
None
40+
dirs::data_dir().and_then(|path| path.parent().map(|parent| parent.join("Jupyter")))
5241
}

crates/ark/src/repos.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -278,14 +278,14 @@ fn get_ppm_linux_repo(repo_url: Option<url::Url>, linux_name: String) -> anyhow:
278278
fn get_p3m_linux_codename(id: String, version: String, version_codename: String) -> String {
279279
// For Debian and Ubuntu, we can just use the codename
280280
if id == "debian" || id == "ubuntu" || id == "pop" {
281-
return version_codename.to_string();
281+
version_codename.to_string()
282282
} else if id == "rhel" {
283283
// For RHEL, we use the id and major version number
284284
let parts: Vec<&str> = version.split('.').collect();
285285
if parts.len() > 1 {
286-
return format!("{}{}", id, parts[0]);
286+
format!("{}{}", id, parts[0])
287287
} else {
288-
return format!("{}{}", id, version);
288+
format!("{}{}", id, version)
289289
}
290290
} else if id == "sles" || id.starts_with("opensuse") {
291291
// For sles and opensuse we use the id and major and minor version number
@@ -294,12 +294,12 @@ fn get_p3m_linux_codename(id: String, version: String, version_codename: String)
294294

295295
let parts: Vec<&str> = version.split('.').collect();
296296
if parts.len() > 1 {
297-
return format!("{}{}{}", distro_id, parts[0], parts[1]);
297+
format!("{}{}{}", distro_id, parts[0], parts[1])
298298
} else {
299-
return format!("{}{}", distro_id, version);
299+
format!("{}{}", distro_id, version)
300300
}
301301
} else {
302-
return String::new();
302+
String::new()
303303
}
304304
}
305305

@@ -322,7 +322,7 @@ fn get_ppm_binary_package_repo(repo_url: Option<url::Url>) -> String {
322322
if let Ok(file) = File::open("/etc/os-release") {
323323
let reader = BufReader::new(file);
324324

325-
for line in reader.lines().flatten() {
325+
for line in reader.lines().map_while(Result::ok) {
326326
if version_codename.is_empty() && line.starts_with(version_codename_key) {
327327
version_codename = line[version_codename_key.len()..].to_string();
328328
} else if id.is_empty() && line.starts_with(id_key) {

crates/ark/tests/dap_breakpoints_symlink.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use amalthea::wire::execute_request::JupyterPositronLocation;
1313
use amalthea::wire::execute_request::JupyterPositronPosition;
1414
use amalthea::wire::execute_request::JupyterPositronRange;
1515
use ark_test::DummyArkFrontend;
16+
#[cfg(target_os = "macos")]
1617
use ark_test::SourceFile;
1718
use url::Url;
1819

justfile

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,7 @@ test-verbose:
1111
# Run the insta tests in update mode
1212
test-insta:
1313
cargo insta test --test-runner nextest
14+
15+
# Run clippy
16+
clippy:
17+
cargo clippy --workspace --all-targets --all-features -- -D warnings

rust-toolchain.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
[toolchain]
2+
channel = "1.94"

0 commit comments

Comments
 (0)