Skip to content

Commit

Permalink
Merge pull request #218 from PlasmaFAIR/bugfix/starkind_fix_is_unsafe
Browse files Browse the repository at this point in the history
Bugfix: `star-kind` fix set to unsafe
  • Loading branch information
LiamPattinson authored Dec 10, 2024
2 parents 7aad90e + 462835d commit f43b3bc
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 21 deletions.
13 changes: 9 additions & 4 deletions docs/rules/star-kind.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# star-kind (T021)
Fix is always available.
Fix is sometimes available.

## What it does
Checks for non-standard kind specifiers such as `int*4` or `real*8`
Expand All @@ -11,6 +11,11 @@ avoided. For these cases, consider instead using 'real(real64)' or
module 'iso_fortran_env'. You may also wish to determine kinds using the
built-in functions 'selected_real_kind' and 'selected_int_kind'.

Also prefers the use of `character(len=*)` to
`character*(*)`, as although the latter is permitted by the standard, the former is
more explicit.
Fixes to this rule are considered unsafe, as while `dtype*N` is generally
understood to mean a `dtype` that occupied `N` bytes, this does not necessarily
correspond to `dtype(N)`, which is a `dtype` of 'kind' `N`. For example, the NAG
compiler may be conigured to use a sequential kind system in which `real*8`
corresponds to `real(2)`

In a future version, we hope to upgrade this to a safe fix by use of parameters
in `iso_fortran_env`, as `real*8` should always correspond to `real(real64)`.
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ snapshot_kind: text
},
{
"fullDescription": {
"text": "## What it does\nChecks for non-standard kind specifiers such as `int*4` or `real*8`\n\n## Why is this bad?\nTypes such as 'real*8' or 'integer*4' are not standard Fortran and should be\navoided. For these cases, consider instead using 'real(real64)' or\n'integer(int32)', where 'real64' and 'int32' may be found in the intrinsic\nmodule 'iso_fortran_env'. You may also wish to determine kinds using the\nbuilt-in functions 'selected_real_kind' and 'selected_int_kind'.\n\nAlso prefers the use of `character(len=*)` to\n`character*(*)`, as although the latter is permitted by the standard, the former is\nmore explicit.\n"
"text": "## What it does\nChecks for non-standard kind specifiers such as `int*4` or `real*8`\n\n## Why is this bad?\nTypes such as 'real*8' or 'integer*4' are not standard Fortran and should be\navoided. For these cases, consider instead using 'real(real64)' or\n'integer(int32)', where 'real64' and 'int32' may be found in the intrinsic\nmodule 'iso_fortran_env'. You may also wish to determine kinds using the\nbuilt-in functions 'selected_real_kind' and 'selected_int_kind'.\n\nFixes to this rule are considered unsafe, as while `dtype*N` is generally\nunderstood to mean a `dtype` that occupied `N` bytes, this does not necessarily\ncorrespond to `dtype(N)`, which is a `dtype` of 'kind' `N`. For example, the NAG\ncompiler may be conigured to use a sequential kind system in which `real*8`\ncorresponds to `real(2)`\n\nIn a future version, we hope to upgrade this to a safe fix by use of parameters\nin `iso_fortran_env`, as `real*8` should always correspond to `real(real64)`.\n"
},
"help": {
"text": "'{dtype}{size}' uses non-standard syntax"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ snapshot_kind: text
|
= help: Replace with 'integer(8)'

Safe fix
Unsafe fix
1 |-integer*8 function add_if(x, y, z)
1 |+integer(8) function add_if(x, y, z)
2 2 | integer(kind=2), intent(in) :: x
Expand All @@ -30,7 +30,7 @@ snapshot_kind: text
|
= help: Replace with 'integer(4)'

Safe fix
Unsafe fix
1 1 | integer*8 function add_if(x, y, z)
2 2 | integer(kind=2), intent(in) :: x
3 |- integer *4, intent(in) :: y
Expand All @@ -50,7 +50,7 @@ snapshot_kind: text
|
= help: Replace with 'logical(4)'

Safe fix
Unsafe fix
1 1 | integer*8 function add_if(x, y, z)
2 2 | integer(kind=2), intent(in) :: x
3 3 | integer *4, intent(in) :: y
Expand All @@ -73,7 +73,7 @@ snapshot_kind: text
|
= help: Replace with 'real(8)'

Safe fix
Unsafe fix
2 2 | integer(kind=2), intent(in) :: x
3 3 | integer *4, intent(in) :: y
4 4 | logical* 4, intent(in) :: z
Expand All @@ -94,7 +94,7 @@ snapshot_kind: text
|
= help: Replace with 'real(4)'

Safe fix
Unsafe fix
13 13 | end function add_if
14 14 |
15 15 | subroutine complex_mul(x, real)
Expand All @@ -115,7 +115,7 @@ snapshot_kind: text
|
= help: Replace with 'complex(8)'

Safe fix
Unsafe fix
14 14 |
15 15 | subroutine complex_mul(x, real)
16 16 | real * 4, intent(in) :: x
Expand Down
23 changes: 15 additions & 8 deletions fortitude/src/rules/typing/star_kinds.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::ast::{dtype_is_plain_number, strip_line_breaks, FortitudeNode};
use crate::settings::Settings;
use crate::{AstRule, FromAstNode};
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Fix};
use ruff_diagnostics::{Diagnostic, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_source_file::SourceFile;
use tree_sitter::Node;
Expand All @@ -16,26 +16,33 @@ use tree_sitter::Node;
/// module 'iso_fortran_env'. You may also wish to determine kinds using the
/// built-in functions 'selected_real_kind' and 'selected_int_kind'.
///
/// Also prefers the use of `character(len=*)` to
/// `character*(*)`, as although the latter is permitted by the standard, the former is
/// more explicit.
/// Fixes to this rule are considered unsafe, as while `dtype*N` is generally
/// understood to mean a `dtype` that occupied `N` bytes, this does not necessarily
/// correspond to `dtype(N)`, which is a `dtype` of 'kind' `N`. For example, the NAG
/// compiler may be conigured to use a sequential kind system in which `real*8`
/// corresponds to `real(2)`
///
/// In a future version, we hope to upgrade this to a safe fix by use of parameters
/// in `iso_fortran_env`, as `real*8` should always correspond to `real(real64)`.
#[violation]
pub struct StarKind {
dtype: String,
size: String,
kind: String,
}

impl AlwaysFixableViolation for StarKind {
impl Violation for StarKind {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;

#[derive_message_formats]
fn message(&self) -> String {
let Self { dtype, size, .. } = self;
format!("'{dtype}{size}' uses non-standard syntax")
}

fn fix_title(&self) -> String {
fn fix_title(&self) -> Option<String> {
let Self { dtype, kind, .. } = self;
format!("Replace with '{dtype}({kind})'")
Some(format!("Replace with '{dtype}({kind})'"))
}
}

Expand All @@ -59,7 +66,7 @@ impl AstRule for StarKind {
let literal = kind_node.child_with_name("number_literal")?;
let kind = literal.to_text(src)?.to_string();
let replacement = format!("{dtype}({kind})");
let fix = Fix::safe_edit(node.edit_replacement(replacement));
let fix = Fix::unsafe_edit(node.edit_replacement(replacement));
some_vec![Diagnostic::from_node(Self { dtype, size, kind }, &kind_node).with_fix(fix)]
}

Expand Down
4 changes: 2 additions & 2 deletions fortitude/tests/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ end program
4 | end program
|
[TEMP_FILE] T021 [*] 'logical*4' uses non-standard syntax
[TEMP_FILE] T021 'logical*4' uses non-standard syntax
|
2 | program test
3 | logical*4, parameter :: true = .true.
Expand Down Expand Up @@ -136,7 +136,7 @@ end program
fortitude explain X001,Y002,...
[*] 2 fixable with the `--fix` option.
[*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option).
----- stderr -----
");
Expand Down

0 comments on commit f43b3bc

Please sign in to comment.