From 3f21af6c4f95a8de53d2bc4fb6e356249c62e65e Mon Sep 17 00:00:00 2001 From: Liam Pattinson Date: Thu, 11 Jan 2024 15:34:45 +0000 Subject: [PATCH] Add rule to catch number literal kinds --- README.md | 3 +- src/best_practices.rs | 13 +- src/best_practices/floating_point.rs | 7 +- src/best_practices/implicit_none.rs | 7 +- src/best_practices/kind_numbers.rs | 183 +++++++++++++++++++++ src/best_practices/modules_and_programs.rs | 7 +- src/main.rs | 1 - test.f90 | 8 +- 8 files changed, 206 insertions(+), 23 deletions(-) create mode 100644 src/best_practices/kind_numbers.rs diff --git a/README.md b/README.md index 815a45a..170871f 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,8 @@ $ cargo test ## TODO -- Avoid `real*8`, and `real(8)`. +- Avoid `real*8`. +- Extend `double precision` rule to also capture `double complex` - Rule for floating point number literals without a kind suffix. - Rule for `use module` without an `only` clause. - Command line interface. diff --git a/src/best_practices.rs b/src/best_practices.rs index 164d72b..4270b10 100644 --- a/src/best_practices.rs +++ b/src/best_practices.rs @@ -1,9 +1,10 @@ use crate::rules::{register_rule, Category, Code, Method, Rule, Status}; use std::collections::HashMap; -pub mod floating_point; -pub mod implicit_none; -pub mod modules_and_programs; +mod floating_point; +mod implicit_none; +mod kind_numbers; +mod modules_and_programs; pub fn add_best_practices_rules(registry: &mut HashMap) { for rule in [ @@ -37,6 +38,12 @@ pub fn add_best_practices_rules(registry: &mut HashMap) { floating_point::AVOID_DOUBLE_PRECISION, Status::Standard, ), + Rule::new( + Code::new(Category::BestPractices, 30), + Method::Tree(kind_numbers::avoid_number_literal_kinds), + kind_numbers::AVOID_NUMBER_LITERAL_KINDS, + Status::Standard, + ), ] { register_rule(registry, rule); } diff --git a/src/best_practices/floating_point.rs b/src/best_practices/floating_point.rs index 56a72ee..fbf29b9 100644 --- a/src/best_practices/floating_point.rs +++ b/src/best_practices/floating_point.rs @@ -28,11 +28,8 @@ pub fn avoid_double_precision(code: Code, root: &Node, src: &str) -> Vec { diff --git a/src/best_practices/implicit_none.rs b/src/best_practices/implicit_none.rs index 39307f9..546dde5 100644 --- a/src/best_practices/implicit_none.rs +++ b/src/best_practices/implicit_none.rs @@ -98,11 +98,8 @@ pub fn avoid_superfluous_implicit_none(code: Code, root: &Node, src: &str) -> Ve ); let query = Query::new(fortran_language(), query_txt.as_str()).unwrap(); let mut cursor = tree_sitter::QueryCursor::new(); - for captures in cursor - .matches(&query, *root, src.as_bytes()) - .map(|x| x.captures) - { - for capture in captures { + for match_ in cursor.matches(&query, *root, src.as_bytes()) { + for capture in match_.captures { violations.push(Violation::from_node( &capture.node, code, diff --git a/src/best_practices/kind_numbers.rs b/src/best_practices/kind_numbers.rs new file mode 100644 index 0000000..e9e850f --- /dev/null +++ b/src/best_practices/kind_numbers.rs @@ -0,0 +1,183 @@ +use crate::parser::fortran_language; +use crate::rules::{Code, Violation}; +/// Defines rules that discourage the use of raw number literals as kinds, and the use +/// of non-standard extensions to the language such as the type `real*8`. +use tree_sitter::{Node, Query}; + +pub const AVOID_NUMBER_LITERAL_KINDS: &str = "\ + Rather than setting an intrinsic type's kind using an integer literal, such as + `real(8)` or `integer(kind=4)`, consider setting precision using the built-in + `selected_real_kind` or `selected_int_kind` functions. Although it is widely + believed that `real(8)` represents an 8-byte floating point (and indeed, this is the + case for most compilers and architectures), there is nothing in the standard to + mandate this, and compiler vendors are free to choose any mapping between kind + numbers and machine precision. This may lead to surprising results if your code is + ported to another machine or compiled with a difference compiler. + + For floating point variables, it is recommended to use `real(sp)` (single + precision), `real(dp)` (double precision), and `real(qp)` (quadruple precision), + using: + + ``` + integer, parameter :: sp = selected_real_kind(6, 37) + integer, parameter :: dp = selected_real_kind(15, 307) + integer, parameter :: qp = selected_real_kind(33, 4931) + ``` + + Or alternatively: + + ``` + use, intrinsic :: iso_fortran_env, only: sp => real32, dp => real64, qp => real128 + ``` + + Some prefer to set one precision parameter `wp` (working precision), which is set + in one module and used throughout a project. + + Integer sizes may be set similarly: + + ``` + integer, parameter :: i1 = selected_int_kind(2) ! 8 bits + integer, parameter :: i2 = selected_int_kind(4) ! 16 bits + integer, parameter :: i4 = selected_int_kind(9) ! 32 bits + integer, parameter :: i8 = selected_int_kind(18) ! 64 bits + ``` + + Or: + + ``` + use, intrinsic :: iso_fortran_env, only: i1 => int8, & + i2 => int16, & + i4 => int32, & + i8 => int64 + ``` + + For code that should be compatible with C, you should instead set kinds such as + `real(c_double)` or `integer(c_int)` which may be found in the intrinsic module + `iso_c_binding`."; + +fn literal_kind_err_msg(dtype: &str) -> Option { + let lower = dtype.to_lowercase(); + match lower.as_str() { + "integer" | "logical" => Some(format!( + "Avoid setting {} kind with raw number literals, and instead use a parameter set \ + using 'selected_int_kind' or one of `int8`, `int16`, `int32` or `int64` from the \ + `iso_fortran_env` module", + lower, + )), + "real" | "complex" => Some(format!( + "Avoid setting {} kind with raw number literals, and instead use a parameter set \ + using 'selected_real_kind' or one of `real32`, `real64`, or `real128` from the \ + `iso_fortran_env` module", + lower, + )), + _ => None, + } +} + +pub fn avoid_number_literal_kinds(code: Code, root: &Node, src: &str) -> Vec { + let mut violations = Vec::new(); + + for query_type in ["function_statement", "variable_declaration"] { + // Find intrinstic types annotated with a single number literal, or with a + // 'kind' keyword. This will also pick up characters with a a length specified, + // but we'll skip those later. + let query_txt = format!( + " + ({} + (intrinsic_type) @type + (size + [ + (argument_list (number_literal)) + (argument_list + (keyword_argument + name: (identifier) + value: (number_literal) + ) + ) + ] + ) + )", + query_type, + ); + let query = Query::new(fortran_language(), &query_txt).unwrap(); + let mut cursor = tree_sitter::QueryCursor::new(); + for match_ in cursor.matches(&query, *root, src.as_bytes()) { + for capture in match_.captures { + let txt = capture.node.utf8_text(src.as_bytes()); + match txt { + Ok(x) => { + match literal_kind_err_msg(x) { + Some(y) => { + violations.push(Violation::from_node( + &capture.node, + code, + y.as_str(), + )); + } + None => { + // Do nothing, characters should be handled elsewhere + } + } + } + Err(_) => { + // Skip, non utf8 text should be caught by a different rule + } + } + } + } + } + violations +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::test_utils::test_utils::{test_tree_method, TEST_CODE}; + + #[test] + fn test_double_precision() { + let source = " + integer(8) function add_if(x, y, z) + integer(kind=2), intent(in) :: x + integer(i32), intent(in) :: y + logical(kind=4), intent(in) :: z + + if (x) then + add_if = x + y + else + add_if = x + end if + end function + + subroutine complex_mul(x, y) + real(8), intent(in) :: x + complex(4), intent(inout) :: y + y = y * x + end subroutine + + complex(real64) function complex_add(x, y) + real(real64), intent(in) :: x + complex(kind=4), intent(in) :: y + complex_add = y + x + end function + "; + let expected_violations = [2, 3, 5, 15, 16, 22] + .iter() + .zip([ + "integer", "integer", "logical", "real", "complex", "complex", + ]) + .map(|(line, kind)| { + Violation::new( + *line, + TEST_CODE, + literal_kind_err_msg(kind).unwrap().as_str(), + ) + }) + .collect(); + test_tree_method( + avoid_number_literal_kinds, + source, + Some(expected_violations), + ); + } +} diff --git a/src/best_practices/modules_and_programs.rs b/src/best_practices/modules_and_programs.rs index 6e47955..960cd41 100644 --- a/src/best_practices/modules_and_programs.rs +++ b/src/best_practices/modules_and_programs.rs @@ -14,11 +14,8 @@ pub fn use_modules_and_programs(code: Code, root: &Node, src: &str) -> Vec