Include file permissions in key for cached files

crates/ruff_cli/Cargo.toml

@@ -30,6 +30,7 @@ ruff_python_ast = { path = "../ruff_python_ast" }
 ruff_python_formatter = { path = "../ruff_python_formatter" }
 ruff_text_size = { workspace = true }
 ruff_textwrap = { path = "../ruff_textwrap" }
+ruff_macros = { path = "../ruff_macros" }
 
 annotate-snippets = { version = "0.9.1", features = ["color"] }
 anyhow = { workspace = true }

crates/ruff_cli/src/cache.rs

@@ -164,21 +164,20 @@ impl Cache {
         path.strip_prefix(&self.package.package_root).ok()
     }
 
-    /// Get the cached results for a single file at relative `path`. This uses
-    /// `file_last_modified` to determine if the results are still accurate
+    /// Get the cached results for a single file at relative `path`. This
+    /// uses `key` to determine if the results are still accurate.
     /// (i.e. if the file hasn't been modified since the cached run).
     ///
-    /// This returns `None` if `file_last_modified` differs from the cached
-    /// timestamp or if the cache doesn't contain results for the file.
-    pub(crate) fn get(
-        &self,
-        path: &RelativePath,
-        file_last_modified: SystemTime,
-    ) -> Option<&FileCache> {
+    /// This returns `None` if `key` differs from the cached key or if the
+    /// cache doesn't contain results for the file.
+    pub(crate) fn get<T: CacheKey>(&self, path: &RelativePath, key: &T) -> Option<&FileCache> {
         let file = self.package.files.get(path)?;
 
+        let mut hasher = CacheKeyHasher::new();
+        key.cache_key(&mut hasher);
+
         // Make sure the file hasn't changed since the cached run.
-        if file.last_modified != file_last_modified {
+        if file.key != hasher.finish() {
             return None;
         }
 
@@ -188,10 +187,10 @@ impl Cache {
     }
 
     /// Add or update a file cache at `path` relative to the package root.
-    pub(crate) fn update(
+    pub(crate) fn update<T: CacheKey>(
         &self,
         path: RelativePathBuf,
-        last_modified: SystemTime,
+        key: T,
         messages: &[Message],
         imports: &ImportMap,
     ) {
@@ -218,8 +217,11 @@ impl Cache {
             })
             .collect();
 
+        let mut hasher = CacheKeyHasher::new();
+        key.cache_key(&mut hasher);
+
         let file = FileCache {
-            last_modified,
+            key: hasher.finish(),
             last_seen: AtomicU64::new(self.last_seen_cache),
             imports: imports.clone(),
             messages,
@@ -244,8 +246,8 @@ struct PackageCache {
 /// On disk representation of the cache per source file.
 #[derive(Deserialize, Debug, Serialize)]
 pub(crate) struct FileCache {
-    /// Timestamp when the file was last modified before the (cached) check.
-    last_modified: SystemTime,
+    /// Key that determines if the cached item is still valid.
+    key: u64,
     /// Timestamp when we last linted this file.
     ///
     /// Represented as the number of milliseconds since Unix epoch. This will
@@ -332,7 +334,6 @@ mod tests {
     use std::io::{self, Write};
     use std::path::{Path, PathBuf};
     use std::sync::atomic::AtomicU64;
-    use std::time::SystemTime;
 
     use ruff::settings::{flags, AllSettings};
     use ruff_cache::CACHE_DIR_NAME;
@@ -487,6 +488,11 @@ mod tests {
                 flip_execute_permission_bit(path)?;
                 Ok(flip_execute_permission_bit)
             },
+            #[cfg(windows)]
+            |path| {
+                flip_read_only_permission(path)?;
+                Ok(flip_read_only_permission)
+            },
         ];
 
         #[cfg(unix)]
@@ -498,6 +504,15 @@ mod tests {
             file.set_permissions(PermissionsExt::from_mode(perms.mode() ^ 0o111))
         }
 
+        #[cfg(windows)]
+        #[allow(clippy::items_after_statements)]
+        fn flip_read_only_permission(path: &Path) -> io::Result<()> {
+            let file = fs::OpenOptions::new().write(true).open(path)?;
+            let mut perms = file.metadata()?.permissions();
+            perms.set_readonly(!perms.readonly());
+            file.set_permissions(perms)
+        }
+
         for change_file in tests {
             let cleanup = change_file(&path).unwrap();
 
@@ -546,7 +561,7 @@ mod tests {
         cache.package.files.insert(
             PathBuf::from("old.py"),
             FileCache {
-                last_modified: SystemTime::UNIX_EPOCH,
+                key: 123,
                 last_seen: AtomicU64::new(123),
                 imports: ImportMap::new(),
                 messages: Vec::new(),

crates/ruff_cli/src/diagnostics.rs

@@ -8,11 +8,15 @@ use std::path::Path;
 
 use anyhow::{anyhow, Result};
 use colored::Colorize;
+use filetime::FileTime;
 use log::{debug, error};
 use ruff_text_size::TextSize;
 use rustc_hash::FxHashMap;
 use similar::TextDiff;
+#[cfg(unix)]
+use std::os::unix::fs::PermissionsExt;
 
+use crate::cache::Cache;
 use ruff::fs;
 use ruff::jupyter::Notebook;
 use ruff::linter::{lint_fix, lint_only, FixTable, FixerResult, LinterResult};
@@ -21,11 +25,18 @@ use ruff::message::Message;
 use ruff::pyproject_toml::lint_pyproject_toml;
 use ruff::settings::{flags, AllSettings, Settings};
 use ruff::source_kind::SourceKind;
+use ruff_macros::CacheKey;
 use ruff_python_ast::imports::ImportMap;
 use ruff_python_ast::source_code::{LineIndex, SourceCode, SourceFileBuilder};
 use ruff_python_stdlib::path::{is_jupyter_notebook, is_project_toml};
 
-use crate::cache::Cache;
+#[derive(CacheKey)]
+pub(crate) struct FileCacheKey {
+    /// Timestamp when the file was last modified before the (cached) check.
+    file_last_modified: FileTime,
+    /// Permissions of the file before the (cached) check.
+    file_permissions_mode: u32,
+}
 
 #[derive(Debug, Default, PartialEq)]
 pub(crate) struct Diagnostics {
@@ -115,12 +126,26 @@ pub(crate) fn lint_path(
             let relative_path = cache
                 .relative_path(path)
                 .expect("wrong package cache for file");
-            let last_modified = path.metadata()?.modified()?;
-            if let Some(cache) = cache.get(relative_path, last_modified) {
+
+            // Construct a cache key for the file
+            let metadata = path.metadata()?;
+
+            #[cfg(unix)]
+            let permissions = metadata.permissions().mode();
+            #[cfg(windows)]
+            let permissions: u32 = metadata.permissions().readonly().into();
+            let cache_key = FileCacheKey {
+                file_last_modified: FileTime::from_last_modification_time(&metadata),
+                file_permissions_mode: permissions,
+            };
+
+            if let Some(cache) = cache.get(relative_path, &cache_key) {
                 return Ok(cache.as_diagnostics(path));
             }
 
-            Some((cache, relative_path, last_modified))
+            // Stash the file metadata for later so when we update the cache it reflects the prerun
+            // information
+            Some((cache, relative_path, cache_key))
         }
         _ => None,
     };
@@ -216,15 +241,10 @@ pub(crate) fn lint_path(
 
     let imports = imports.unwrap_or_default();
 
-    if let Some((cache, relative_path, file_last_modified)) = caching {
+    if let Some((cache, relative_path, key)) = caching {
         // We don't cache parsing errors.
         if parse_error.is_none() {
-            cache.update(
-                relative_path.to_owned(),
-                file_last_modified,
-                &messages,
-                &imports,
-            );
+            cache.update(relative_path.to_owned(), key, &messages, &imports);
         }
     }