Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify language default version implementation #150

Merged
merged 3 commits into from
Dec 13, 2024
Merged

Simplify language default version implementation #150

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b07426d
Simplify language default version implementation
j178 Dec 13, 2024
c604dee
Add comments
j178 Dec 13, 2024
3a90b3c
Add test
j178 Dec 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
196 changes: 194 additions & 2 deletions src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,15 @@ impl Language {
Self::System => "system",
}
}

/// Return whether the language allows specifying the version.
/// See <https://pre-commit.com/#overriding-language-version>
pub fn allow_specify_version(self) -> bool {
matches!(
self,
Self::Python | Self::Node | Self::Ruby | Self::Rust | Self::Golang
)
}
}

impl Display for Language {
Expand Down Expand Up @@ -213,7 +222,7 @@ pub struct Config {
/// Default is `[pre-commit]`.
pub default_install_hook_types: Option<Vec<HookType>>,
/// A mapping from language to the default `language_version`.
pub default_language_version: Option<HashMap<Language, String>>,
pub default_language_version: Option<HashMap<Language, LanguageVersion>>,
/// A configuration-wide default for the stages property of hooks.
/// Default to all stages.
pub default_stages: Option<Vec<Stage>>,
Expand Down Expand Up @@ -275,6 +284,56 @@ impl Display for RepoLocation {
}
}

#[derive(Default, Debug, Clone)]
pub enum LanguageVersion {
/// By default, pre-commit will use the system installed version,
/// if not found, it will try to download and install a version.
#[default]
Default,
/// Use the system installed version.
System,
/// Download and install a specific version.
Specific(String),
}

impl<'de> Deserialize<'de> for LanguageVersion {
fn deserialize<D>(deserializer: D) -> Result<LanguageVersion, D::Error>
where
D: Deserializer<'de>,
{
let s = String::deserialize(deserializer)?;
match &*s {
"default" => Ok(LanguageVersion::Default),
"system" => Ok(LanguageVersion::System),
_ => Ok(LanguageVersion::Specific(s)),
}
}
}

impl LanguageVersion {
pub fn is_default(&self) -> bool {
matches!(self, Self::Default)
}

pub fn is_system(&self) -> bool {
matches!(self, Self::System)
}

pub fn as_str(&self) -> &str {
match self {
Self::Default => "default",
Self::System => "system",
Self::Specific(s) => s,
}
}
}

impl Display for LanguageVersion {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str(self.as_str())
}
}

/// Common hook options.
#[derive(Debug, Clone, Default, Deserialize)]
pub struct HookOptions {
Expand Down Expand Up @@ -312,7 +371,7 @@ pub struct HookOptions {
/// Run the hook on a specific version of the language.
/// Default is `default`.
/// See <https://pre-commit.com/#overriding-language-version>.
pub language_version: Option<String>,
pub language_version: Option<LanguageVersion>,
/// Write the output of the hook to a file when the hook fails or verbose is enabled.
pub log_file: Option<String>,
/// This hook will execute using a single process instead of in parallel.
Expand Down Expand Up @@ -1113,6 +1172,139 @@ mod tests {
"#);
}

#[test]
fn language_version() {
let yaml = indoc::indoc! { r"
repos:
- repo: local
hooks:
- id: hook-1
name: hook 1
entry: echo hello world
language: system
language_version: default
- id: hook-2
name: hook 2
entry: echo hello world
language: system
language_version: system
- id: hook-3
name: hook 3
entry: echo hello world
language: system
language_version: '3.8'
"};
let result = serde_yaml::from_str::<Config>(yaml);
insta::assert_debug_snapshot!(result, @r#"
Ok(
Config {
repos: [
Local(
LocalRepo {
hooks: [
ManifestHook {
id: "hook-1",
name: "hook 1",
entry: "echo hello world",
language: System,
options: HookOptions {
alias: None,
files: None,
exclude: None,
types: None,
types_or: None,
exclude_types: None,
additional_dependencies: None,
args: None,
always_run: None,
fail_fast: None,
pass_filenames: None,
description: None,
language_version: Some(
Default,
),
log_file: None,
require_serial: None,
stages: None,
verbose: None,
minimum_pre_commit_version: None,
},
},
ManifestHook {
id: "hook-2",
name: "hook 2",
entry: "echo hello world",
language: System,
options: HookOptions {
alias: None,
files: None,
exclude: None,
types: None,
types_or: None,
exclude_types: None,
additional_dependencies: None,
args: None,
always_run: None,
fail_fast: None,
pass_filenames: None,
description: None,
language_version: Some(
System,
),
log_file: None,
require_serial: None,
stages: None,
verbose: None,
minimum_pre_commit_version: None,
},
},
ManifestHook {
id: "hook-3",
name: "hook 3",
entry: "echo hello world",
language: System,
options: HookOptions {
alias: None,
files: None,
exclude: None,
types: None,
types_or: None,
exclude_types: None,
additional_dependencies: None,
args: None,
always_run: None,
fail_fast: None,
pass_filenames: None,
description: None,
language_version: Some(
Specific(
"3.8",
),
),
log_file: None,
require_serial: None,
stages: None,
verbose: None,
minimum_pre_commit_version: None,
},
},
],
},
),
],
default_install_hook_types: None,
default_language_version: None,
default_stages: None,
files: None,
exclude: None,
fail_fast: None,
minimum_pre_commit_version: None,
ci: None,
},
)
"#);
}

#[test]
fn test_read_config() -> Result<()> {
let config = read_config(Path::new("tests/files/uv-pre-commit-config.yaml"))?;
Expand Down
45 changes: 25 additions & 20 deletions src/hook.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,10 @@ use tracing::{debug, error};
use url::Url;

use crate::config::{
self, read_config, read_manifest, Config, Language, LocalHook, ManifestHook, MetaHook,
RemoteHook, Stage, CONFIG_FILE, MANIFEST_FILE,
self, read_config, read_manifest, Config, Language, LanguageVersion, LocalHook, ManifestHook,
MetaHook, RemoteHook, Stage, CONFIG_FILE, MANIFEST_FILE,
};
use crate::fs::{Simplified, CWD};
use crate::languages::DEFAULT_VERSION;
use crate::store::Store;
use crate::warn_user;

Expand Down Expand Up @@ -363,9 +362,6 @@ impl HookBuilder {
.as_ref()
.and_then(|v| v.get(&language).cloned());
}
if options.language_version.is_none() {
options.language_version = Some(language.default_version().to_string());
}

if options.stages.is_none() {
options.stages.clone_from(&config.default_stages);
Expand All @@ -375,14 +371,12 @@ impl HookBuilder {
/// Fill in the default values for the hook configuration.
fn fill_in_defaults(&mut self) {
let options = &mut self.config.options;
options
.language_version
.get_or_insert(DEFAULT_VERSION.to_string());
options.alias.get_or_insert(String::new());
options.args.get_or_insert(Vec::new());
options.language_version.get_or_insert_default();
options.alias.get_or_insert_default();
options.args.get_or_insert_default();
options.types.get_or_insert(vec!["file".to_string()]);
options.types_or.get_or_insert(Vec::new());
options.exclude_types.get_or_insert(Vec::new());
options.types_or.get_or_insert_default();
options.exclude_types.get_or_insert_default();
options.always_run.get_or_insert(false);
options.fail_fast.get_or_insert(false);
options.pass_filenames.get_or_insert(true);
Expand All @@ -391,23 +385,34 @@ impl HookBuilder {
options
.stages
.get_or_insert(Stage::value_variants().to_vec());
options.additional_dependencies.get_or_insert(Vec::new());
options.additional_dependencies.get_or_insert_default();
}

/// Check the hook configuration.
fn check(&self) {
let language = self.config.language;
let options = &self.config.options;
if language.environment_dir().is_none() {
if self.config.options.language_version != Some(DEFAULT_VERSION.to_string()) {
if options.additional_dependencies.is_some() {
warn_user!(
"Language {} does not need environment, but language_version is set",
"Language {} does not need environment, but additional_dependencies is set",
language
);
}

if self.config.options.additional_dependencies.is_some() {
}
if options
.language_version
.as_ref()
.is_some_and(|v| !v.is_default())
{
if language.environment_dir().is_none() {
warn_user!(
"Language {} does not need environment, but additional_dependencies is set",
"Language {} does not need environment, but language_version is set",
language
);
} else if !language.allow_specify_version() {
warn_user!(
"Language {} does not support specifying version, but language_version is set",
language
);
}
Expand Down Expand Up @@ -473,7 +478,7 @@ pub struct Hook {
pub fail_fast: bool,
pub pass_filenames: bool,
pub description: Option<String>,
pub language_version: String,
pub language_version: LanguageVersion,
pub log_file: Option<String>,
pub require_serial: bool,
pub stages: Vec<Stage>,
Expand Down
6 changes: 1 addition & 5 deletions src/languages/docker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use tracing::trace;

use crate::fs::CWD;
use crate::hook::Hook;
use crate::languages::{LanguageImpl, DEFAULT_VERSION};
use crate::languages::LanguageImpl;
use crate::process::Cmd;
use crate::run::run_by_batch;

Expand Down Expand Up @@ -161,10 +161,6 @@ impl Docker {
}

impl LanguageImpl for Docker {
fn default_version(&self) -> &str {
DEFAULT_VERSION
}

fn environment_dir(&self) -> Option<&str> {
Some("docker")
}
Expand Down
6 changes: 1 addition & 5 deletions src/languages/docker_image.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,13 @@ use std::sync::Arc;

use crate::hook::Hook;
use crate::languages::docker::Docker;
use crate::languages::{LanguageImpl, DEFAULT_VERSION};
use crate::languages::LanguageImpl;
use crate::run::run_by_batch;

#[derive(Debug, Copy, Clone)]
pub struct DockerImage;

impl LanguageImpl for DockerImage {
fn default_version(&self) -> &str {
DEFAULT_VERSION
}

fn environment_dir(&self) -> Option<&str> {
None
}
Expand Down
6 changes: 1 addition & 5 deletions src/languages/fail.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
use std::{collections::HashMap, sync::Arc};

use crate::hook::Hook;
use crate::languages::{LanguageImpl, DEFAULT_VERSION};
use crate::languages::LanguageImpl;

#[derive(Debug, Copy, Clone)]
pub struct Fail;

impl LanguageImpl for Fail {
fn default_version(&self) -> &str {
DEFAULT_VERSION
}

fn environment_dir(&self) -> Option<&str> {
None
}
Expand Down
Loading
Loading