From 38d56e4378994109813cbfe5374c413be10540e1 Mon Sep 17 00:00:00 2001 From: Ian McInerney Date: Thu, 26 Dec 2024 15:40:18 +0000 Subject: [PATCH 1/2] Add check for use of specific names for intrinsic functions --- .../test/fixtures/obsolescent/OB031.f90 | 21 ++++ fortitude/src/rules/mod.rs | 1 + fortitude/src/rules/obsolescent/mod.rs | 2 + ...cent__tests__specific-names_OB031.f90.snap | 79 ++++++++++++++ .../src/rules/obsolescent/specific_names.rs | 100 ++++++++++++++++++ 5 files changed, 203 insertions(+) create mode 100644 fortitude/resources/test/fixtures/obsolescent/OB031.f90 create mode 100644 fortitude/src/rules/obsolescent/snapshots/fortitude__rules__obsolescent__tests__specific-names_OB031.f90.snap create mode 100644 fortitude/src/rules/obsolescent/specific_names.rs diff --git a/fortitude/resources/test/fixtures/obsolescent/OB031.f90 b/fortitude/resources/test/fixtures/obsolescent/OB031.f90 new file mode 100644 index 00000000..65cb66f0 --- /dev/null +++ b/fortitude/resources/test/fixtures/obsolescent/OB031.f90 @@ -0,0 +1,21 @@ +subroutine test() + use, intrinsic :: iso_fortran_env, dp => real64 + real(kind=dp) :: x, y + + y = ASIN(x) + y = DSIN(x) +end subroutine test + +subroutine test1() + use, intrinsic :: iso_fortran_env, dp => real64 + real(kind=dp) :: x, y + + y = ASIN(x) + DSIN(x) +end subroutine test1 + +subroutine test2() + use, intrinsic :: iso_fortran_env, dp => real64 + real(kind=dp) :: x, y + + y = dsin(x) + dcos(x) +end subroutine test2 diff --git a/fortitude/src/rules/mod.rs b/fortitude/src/rules/mod.rs index 2b17da92..25326454 100644 --- a/fortitude/src/rules/mod.rs +++ b/fortitude/src/rules/mod.rs @@ -104,6 +104,7 @@ pub fn code_to_rule(category: Category, code: &str) -> Option<(RuleGroup, Rule)> (Obsolescent, "001") => (RuleGroup::Stable, Ast, obsolescent::statement_functions::StatementFunction), (Obsolescent, "011") => (RuleGroup::Stable, Ast, obsolescent::common_blocks::CommonBlock), (Obsolescent, "021") => (RuleGroup::Stable, Ast, obsolescent::entry_statement::EntryStatement), + (Obsolescent, "031") => (RuleGroup::Preview, Ast, obsolescent::specific_names::SpecificNames), (Precision, "001") => (RuleGroup::Stable, Ast, precision::kind_suffixes::NoRealSuffix), (Precision, "011") => (RuleGroup::Stable, Ast, precision::double_precision::DoublePrecision), diff --git a/fortitude/src/rules/obsolescent/mod.rs b/fortitude/src/rules/obsolescent/mod.rs index b4d57156..b2b5666f 100644 --- a/fortitude/src/rules/obsolescent/mod.rs +++ b/fortitude/src/rules/obsolescent/mod.rs @@ -1,5 +1,6 @@ pub mod common_blocks; pub mod entry_statement; +pub mod specific_names; pub mod statement_functions; #[cfg(test)] @@ -18,6 +19,7 @@ mod tests { #[test_case(Rule::StatementFunction, Path::new("OB001.f90"))] #[test_case(Rule::CommonBlock, Path::new("OB011.f90"))] #[test_case(Rule::EntryStatement, Path::new("OB021.f90"))] + #[test_case(Rule::SpecificNames, Path::new("OB031.f90"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/fortitude/src/rules/obsolescent/snapshots/fortitude__rules__obsolescent__tests__specific-names_OB031.f90.snap b/fortitude/src/rules/obsolescent/snapshots/fortitude__rules__obsolescent__tests__specific-names_OB031.f90.snap new file mode 100644 index 00000000..3cc1336d --- /dev/null +++ b/fortitude/src/rules/obsolescent/snapshots/fortitude__rules__obsolescent__tests__specific-names_OB031.f90.snap @@ -0,0 +1,79 @@ +--- +source: fortitude/src/rules/obsolescent/mod.rs +expression: diagnostics +snapshot_kind: text +--- +./resources/test/fixtures/obsolescent/OB031.f90:6:9: OB031 [*] deprecated specific name 'DSIN', prefer 'SIN' instead + | +5 | y = ASIN(x) +6 | y = DSIN(x) + | ^^^^ OB031 +7 | end subroutine test + | + = help: Use 'SIN' + +ℹ Safe fix +3 3 | real(kind=dp) :: x, y +4 4 | +5 5 | y = ASIN(x) +6 |- y = DSIN(x) + 6 |+ y = SIN(x) +7 7 | end subroutine test +8 8 | +9 9 | subroutine test1() + +./resources/test/fixtures/obsolescent/OB031.f90:13:19: OB031 [*] deprecated specific name 'DSIN', prefer 'SIN' instead + | +11 | real(kind=dp) :: x, y +12 | +13 | y = ASIN(x) + DSIN(x) + | ^^^^ OB031 +14 | end subroutine test1 + | + = help: Use 'SIN' + +ℹ Safe fix +10 10 | use, intrinsic :: iso_fortran_env, dp => real64 +11 11 | real(kind=dp) :: x, y +12 12 | +13 |- y = ASIN(x) + DSIN(x) + 13 |+ y = ASIN(x) + SIN(x) +14 14 | end subroutine test1 +15 15 | +16 16 | subroutine test2() + +./resources/test/fixtures/obsolescent/OB031.f90:20:9: OB031 [*] deprecated specific name 'DSIN', prefer 'SIN' instead + | +18 | real(kind=dp) :: x, y +19 | +20 | y = dsin(x) + dcos(x) + | ^^^^ OB031 +21 | end subroutine test2 + | + = help: Use 'SIN' + +ℹ Safe fix +17 17 | use, intrinsic :: iso_fortran_env, dp => real64 +18 18 | real(kind=dp) :: x, y +19 19 | +20 |- y = dsin(x) + dcos(x) + 20 |+ y = SIN(x) + dcos(x) +21 21 | end subroutine test2 + +./resources/test/fixtures/obsolescent/OB031.f90:20:19: OB031 [*] deprecated specific name 'DCOS', prefer 'COS' instead + | +18 | real(kind=dp) :: x, y +19 | +20 | y = dsin(x) + dcos(x) + | ^^^^ OB031 +21 | end subroutine test2 + | + = help: Use 'COS' + +ℹ Safe fix +17 17 | use, intrinsic :: iso_fortran_env, dp => real64 +18 18 | real(kind=dp) :: x, y +19 19 | +20 |- y = dsin(x) + dcos(x) + 20 |+ y = dsin(x) + COS(x) +21 21 | end subroutine test2 diff --git a/fortitude/src/rules/obsolescent/specific_names.rs b/fortitude/src/rules/obsolescent/specific_names.rs new file mode 100644 index 00000000..f9312b5c --- /dev/null +++ b/fortitude/src/rules/obsolescent/specific_names.rs @@ -0,0 +1,100 @@ +use crate::ast::FortitudeNode; +use crate::settings::Settings; +use crate::{AstRule, FromAstNode}; +use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Fix}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_source_file::SourceFile; +use tree_sitter::Node; + +fn map_specific_intrinsic_functions(name: &str) -> Option<&'static str> { + match name { + // Real-specific functions + "ALOG" => Some("LOG"), + "ALOG10" => Some("LOG10"), + "AMOD" => Some("MOD"), + + "AMAX1" => Some("MAX"), + "AMIN1" => Some("MIN"), + + // Complex-specific functions + "CABS" => Some("ABS"), + "CCOS" => Some("COS"), + "CEXP" => Some("EXP"), + "CLOG" => Some("LOG"), + "CSIN" => Some("SIN"), + "CSQRT" => Some("SQRT"), + + // Double precision-specific functions + "DABS" => Some("ABS"), + "DACOS" => Some("ACOS"), + "DASIN" => Some("ASIN"), + "DATAN" => Some("ATAN"), + "DATAN2" => Some("ATAN2"), + "DCOS" => Some("COS"), + "DCOSH" => Some("COSH"), + "DDIM" => Some("DIM"), + "DEXP" => Some("EXP"), + "DINT" => Some("AINT"), + "DLOG" => Some("LOG"), + "DLOG10" => Some("LOG10"), + "DMOD" => Some("MOD"), + "DNINT" => Some("ANINT"), + "DSIGN" => Some("SIGN"), + "DSIN" => Some("SIN"), + "DSINH" => Some("SINH"), + "DSQRT" => Some("SQRT"), + "DTAN" => Some("TAN"), + "DTANH" => Some("TANH"), + "IDNINT" => Some("NINT"), + + // Integer-specific functions + "IABS" => Some("ABS"), + "IDIM" => Some("DIM"), + "ISIGN" => Some("SIGN"), + _ => None, + } +} + +/// ## What does it do? +/// Checks for uses of the deprecated specific names of intrinsic functions. +/// +/// ## Why is this bad? +/// Specific names of intrinsic functions can be obscure and hinder readability of +/// the code. Fortran 90 made these specific names redundant and recommends the use +/// of the generic names for calling intrinsic functions. +#[violation] +pub struct SpecificNames { + func: String, + new_func: String, +} + +impl AlwaysFixableViolation for SpecificNames { + #[derive_message_formats] + fn message(&self) -> String { + let Self { func, new_func } = self; + format!("deprecated specific name '{func}', prefer '{new_func}' instead") + } + + fn fix_title(&self) -> String { + let Self { new_func, .. } = self; + format!("Use '{new_func}'") + } +} + +impl AstRule for SpecificNames { + fn check(_settings: &Settings, node: &Node, src: &SourceFile) -> Option> { + let name_node = node.child_with_name("identifier")?; + let func = name_node + .to_text(src.source_text())? + .to_uppercase() + .to_string(); + let new_func = map_specific_intrinsic_functions(func.as_str())?.to_string(); + let fix = Fix::safe_edit(name_node.edit_replacement(src, new_func.clone())); + + some_vec![Diagnostic::from_node(Self { func, new_func }, &name_node).with_fix(fix)] + } + + fn entrypoints() -> Vec<&'static str> { + vec!["call_expression"] + } +} From 62332e6fcdbcb417217fce8701c393b4df16609f Mon Sep 17 00:00:00 2001 From: Ian McInerney Date: Tue, 7 Jan 2025 18:05:38 +0000 Subject: [PATCH 2/2] Match case of the original intrinsic in the fix --- fortitude/src/rules/mod.rs | 1 + ...cent__tests__specific-names_OB031.f90.snap | 13 ++++++------ .../src/rules/obsolescent/specific_names.rs | 21 ++++++++++++------- fortitude/src/rules/utilities.rs | 9 ++++++++ 4 files changed, 30 insertions(+), 14 deletions(-) create mode 100644 fortitude/src/rules/utilities.rs diff --git a/fortitude/src/rules/mod.rs b/fortitude/src/rules/mod.rs index 25326454..199aab96 100644 --- a/fortitude/src/rules/mod.rs +++ b/fortitude/src/rules/mod.rs @@ -13,6 +13,7 @@ pub(crate) mod readability; pub(crate) mod style; pub(crate) mod testing; pub(crate) mod typing; +pub mod utilities; use crate::registry::{AsRule, Category}; use std::fmt::Formatter; diff --git a/fortitude/src/rules/obsolescent/snapshots/fortitude__rules__obsolescent__tests__specific-names_OB031.f90.snap b/fortitude/src/rules/obsolescent/snapshots/fortitude__rules__obsolescent__tests__specific-names_OB031.f90.snap index 3cc1336d..e4ebe985 100644 --- a/fortitude/src/rules/obsolescent/snapshots/fortitude__rules__obsolescent__tests__specific-names_OB031.f90.snap +++ b/fortitude/src/rules/obsolescent/snapshots/fortitude__rules__obsolescent__tests__specific-names_OB031.f90.snap @@ -1,7 +1,6 @@ --- source: fortitude/src/rules/obsolescent/mod.rs expression: diagnostics -snapshot_kind: text --- ./resources/test/fixtures/obsolescent/OB031.f90:6:9: OB031 [*] deprecated specific name 'DSIN', prefer 'SIN' instead | @@ -42,7 +41,7 @@ snapshot_kind: text 15 15 | 16 16 | subroutine test2() -./resources/test/fixtures/obsolescent/OB031.f90:20:9: OB031 [*] deprecated specific name 'DSIN', prefer 'SIN' instead +./resources/test/fixtures/obsolescent/OB031.f90:20:9: OB031 [*] deprecated specific name 'dsin', prefer 'sin' instead | 18 | real(kind=dp) :: x, y 19 | @@ -50,17 +49,17 @@ snapshot_kind: text | ^^^^ OB031 21 | end subroutine test2 | - = help: Use 'SIN' + = help: Use 'sin' ℹ Safe fix 17 17 | use, intrinsic :: iso_fortran_env, dp => real64 18 18 | real(kind=dp) :: x, y 19 19 | 20 |- y = dsin(x) + dcos(x) - 20 |+ y = SIN(x) + dcos(x) + 20 |+ y = sin(x) + dcos(x) 21 21 | end subroutine test2 -./resources/test/fixtures/obsolescent/OB031.f90:20:19: OB031 [*] deprecated specific name 'DCOS', prefer 'COS' instead +./resources/test/fixtures/obsolescent/OB031.f90:20:19: OB031 [*] deprecated specific name 'dcos', prefer 'cos' instead | 18 | real(kind=dp) :: x, y 19 | @@ -68,12 +67,12 @@ snapshot_kind: text | ^^^^ OB031 21 | end subroutine test2 | - = help: Use 'COS' + = help: Use 'cos' ℹ Safe fix 17 17 | use, intrinsic :: iso_fortran_env, dp => real64 18 18 | real(kind=dp) :: x, y 19 19 | 20 |- y = dsin(x) + dcos(x) - 20 |+ y = dsin(x) + COS(x) + 20 |+ y = dsin(x) + cos(x) 21 21 | end subroutine test2 diff --git a/fortitude/src/rules/obsolescent/specific_names.rs b/fortitude/src/rules/obsolescent/specific_names.rs index f9312b5c..bdf183af 100644 --- a/fortitude/src/rules/obsolescent/specific_names.rs +++ b/fortitude/src/rules/obsolescent/specific_names.rs @@ -1,4 +1,5 @@ use crate::ast::FortitudeNode; +use crate::rules::utilities; use crate::settings::Settings; use crate::{AstRule, FromAstNode}; use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Fix}; @@ -84,14 +85,20 @@ impl AlwaysFixableViolation for SpecificNames { impl AstRule for SpecificNames { fn check(_settings: &Settings, node: &Node, src: &SourceFile) -> Option> { let name_node = node.child_with_name("identifier")?; - let func = name_node - .to_text(src.source_text())? - .to_uppercase() - .to_string(); - let new_func = map_specific_intrinsic_functions(func.as_str())?.to_string(); - let fix = Fix::safe_edit(name_node.edit_replacement(src, new_func.clone())); + let func = name_node.to_text(src.source_text())?; - some_vec![Diagnostic::from_node(Self { func, new_func }, &name_node).with_fix(fix)] + let new_func = map_specific_intrinsic_functions(func.to_uppercase().as_str())?; + let matched_case = utilities::match_original_case(func, new_func)?; + + let fix = Fix::safe_edit(name_node.edit_replacement(src, matched_case.clone())); + some_vec![Diagnostic::from_node( + Self { + func: func.to_string(), + new_func: matched_case + }, + &name_node + ) + .with_fix(fix)] } fn entrypoints() -> Vec<&'static str> { diff --git a/fortitude/src/rules/utilities.rs b/fortitude/src/rules/utilities.rs new file mode 100644 index 00000000..b3e014dc --- /dev/null +++ b/fortitude/src/rules/utilities.rs @@ -0,0 +1,9 @@ +pub fn match_original_case(original: &str, new: &str) -> Option { + let first_ch = original.chars().next()?; + + if first_ch.is_lowercase() { + Some(new.to_lowercase()) + } else { + Some(new.to_uppercase()) + } +}