FluidNumerics / FluidNumerics/SELF

Bring the tree into line with the new style checks: 461 pre-existing violations

Open
#187 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Fortran
Stars
92
Forks
13
Avg merge
20h 38m
Merged PRs (30d)
7

Description

The style checks introduced in #186 land on a tree that predates them, so they report 461 pre-existing violations across 91 files. #186 deliberately fixes none of them: it introduces the checks and records the backlog. This issue is that backlog.

Regenerate these numbers at any time with:

```shell
python3 .github/scripts/style_check.py # src/, test/, examples/
python3 .github/scripts/docs_style_check.py # docs/ and the root pages
```

Rule descriptions are in `docs/Contributing/StyleGuide.md`.

## Summary

| rule | what | violations | files | risk |
|---|---|---|---|---|
| F001 | licence banner absent, truncated or altered | 18 | 18 | none, comment text |
| F002 | doxygen `!>` predocmark instead of FORD `!!` | 14 | 1 | none, comment text |
| F003 | typographic punctuation in comments | 26 | 11 | none, comment text |
| F004 | markdown inside comments | 17 | 6 | none, comment text |
| F007 | missing `implicit none` | 109 | 35 | **can break the build** |
| F102 | comment line over 132 characters | 5 | 5 | none, comment text |
| D001 | pictographs in documentation | 9 | 1 | none |
| D002 | typographic punctuation in documentation | 248 | 14 | none |
| D003 | page without a title | 4 | 4 | none |
| D006 | page absent from the mkdocs nav tree | 8 | 8 | none |
| D100 | emphasis density above the calibrated limit | 3 | 3 | none |

352 of the 461 are comment or documentation text and cannot change compiled output. I verified this rather than assuming it: none of the 26 F003 hits are in code, all are inside comments. The remaining 109 are F007 and need a different kind of care.

For reference, the same checks run against `f3e1e57c`, the last commit before the first AI assisted contribution, report 106 violations: 9 F001, 14 F002, 68 F007, 1 F102, 4 D002, 4 D003 and 6 D006. That tree has 204 Fortran files and 31 documentation pages against 448 and 45 now, so it is not a like for like comparison, but two things are clear from it.

F007 and F002 are long standing. 68 of the 109 missing `implicit none` and all 14 doxygen predocmarks were already there. Seven of the eighteen damaged licence banners were too.

F003, F004, D001 and D100 are entirely new, and D002 has gone from 4 to 248. These are the rules that measure prose style, which is what the checks were built to catch.

## Suggested sequencing

Three separate pull requests, in this order. They have different risk profiles and mixing them would bury the risky changes in several hundred lines of text edits.

### 1. Licence banners (F001), 18 files

Highest priority and smallest change. This is a legal text problem rather than a style one, and it was invisible before the canonical comparison in #186.

Fix by copying `.github/license-header.txt` verbatim.

**Missing entirely (1)**

- `examples/esatmo3d_abc_flow.f90`

**Truncated to 18 lines, the entire warranty disclaimer deleted (10)**

- `examples/linear_euler2d_boneandmarrow.f90`
- `examples/linear_euler2d_pml_planewave.f90`
- `test/mesh2d_read_ism.f90`
- `test/mesh2d_read_ismmm.f90`
- `test/mesh3d_read_guard_badconnectivity.f90`
- `test/mesh3d_read_guard_missingfile.f90`
- `test/mesh3d_read_ism.f90`
- `test/mesh3d_read_ismmm.f90`
- `test/mesh3d_read_manymaterials.f90`
- `test/mesh3d_read_rotatedstack.f90`

These are missing the paragraph beginning "THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS" through to the end of the limitation of liability. That is the whole warranty disclaimer and limitation of liability, so these files are not carrying a complete BSD-3 licence.

**Text corrupted (7)**

- `src/SELF_Burgers1D_t.f90`
- `src/SELF_LinearEuler2D_t.f90`
- `src/SELF_LinearEuler3D_t.f90`
- `src/SELF_NullDGModel1D_t.f90`
- `src/SELF_NullDGModel2D_t.f90`
- `src/SELF_NullDGModel3D_t.f90`
- `src/SELF_advection_diffusion_1d_t.f90`

All seven share one signature: `BUSINESS` reads `BUsLESS` and `ARISING` reads `ARIsLG`, which is a global replacement of `SIN` with `sL`. It looks like a scripted edit that was meant for something else and was applied to these files by mistake.

I checked whether that edit damaged anything beyond the licence text and it did not. Every other occurrence of the pattern in the tree is a legitimate identifier, such as `nBCsLocal` in `src/SELF_Mesh_2D_t.f90` and the `sL`/`sR` left and right states in the Riemann solvers. The damage is confined to the banners.

### 2. Comment and documentation text, 334 violations

Large diff, no risk, one review pass. Nothing here can change compiled output.

**F003 and F004**, typographic punctuation and markdown in Fortran comments, 43 violations across 17 files:

- `examples/esatmo2d_thermal_bubble.f90`
- `examples/esatmo3d_thermal_bubble.f90`
- `src/gpu/SELF_ECAdvection2D.f90`
- `src/gpu/SELF_ECAdvection3D.f90`
- `src/gpu/SELF_Points.f90`
- `src/SELF_ECDGModel2D_t.f90`
- `src/SELF_ESAtmo2D_t.f90`
- `src/SELF_ESAtmo3D_t.f90`
- `src/SELF_Points_t.f90`
- `src/SELF_RefinementIndicator_2D_t.f90`
- `src/SELF_RefinementIndicator_3D_t.f90`
- `src/SELF_RefinementPrimitives_2D.f90`
- `src/SELF_RefinementPrimitives_3D.f90`
- `src/SELF_SolutionTransfer_2D.f90`
- `src/SELF_SolutionTransfer_3D.f90`
- `test/ec_advection_2d_entropy_conservation.f90`
- `test/ec_advection_3d_entropy_conservation.f90`

**F002**, 14 doxygen `!>` predocmarks in `src/SELF_SupportRoutines.f90`. This file is a holdover from an earlier documentation convention; converting it to FORD post-marks would also let its `

` and `` HTML go.

**F102**, 5 comment lines over 132 characters.

**D002**, 248 typographic punctuation marks across 14 documentation pages, heavily concentrated:

- `docs/Learning/AdaptiveMeshRefinement.md`, 77
- `docs/Contributing/AMR3D-Design.md`, 37
- `docs/Contributing/AMR2D-Design.md`, 37
- `docs/Contributing/Mortar2D-Design.md`, 21
- `docs/Models/linear-euler-2d-pml-model.md`, 18
- `docs/Models/nonconforming-mortar-interfaces.md`, 12
- `docs/Models/linear-euler-2d-model.md`, 11
- `docs/Tutorials/LinearEuler2D/PerfectlyMatchedLayer.md`, 8
- `docs/Models/linear-euler-3d-model.md`, 8
- `docs/Learning/PointSampling.md`, 8
- `docs/Models/model-hooks.md`, 4
- `docs/Learning/DifferentialGeometry.md`, 3
- `docs/GettingStarted/multi-gpu.md`, 3
- `docs/MeshGeneration/Overview.md`, 1

**D001**, 9 status pictographs in the tables of `docs/Contributing/AMR2D-Design.md`.

**D100**, 3 pages above the emphasis density limit: `docs/Models/boundary-conditions.md`, `docs/Learning/BoundaryConditions.md`, `docs/Learning/SplitFormDGSEM.md`.

**D003 and D006**, structural. Four pages have no title and eight are absent from the `mkdocs.yml` nav tree, so they are published but unreachable. Six of the orphans predate the AI assisted work. Some may be intentionally unpublished drafts, in which case the fix is to delete them or move them out of `docs/`, not to add nav entries:

- `docs/Contributing/Overview.md`
- `docs/Learning/ShallowWaterSplitForm.md`
- `docs/Learning/SplitFormDGSEM.md`
- `docs/MeshGeneration/HOPr.md`
- `docs/Models/generic-dg-models.md`
- `docs/PostProcessing/VisualizationWithPyself.md`
- `docs/Tutorials/CreateYourOwnModel.md`
- `docs/Tutorials/LinearShallowWater/ReflectingWave.md`

### 3. `implicit none` (F007), 109 violations across 35 files

This one is not mechanical and should be split by directory, with compiler matrix results in each pull request.

`CLAUDE.md` section 2 requires `implicit none` in all program units. Adding it to a scope that has been relying on implicit typing will fail to compile, and that failure is the point: each one is a latent bug where a variable was never declared. Expect to find real defects here rather than a clean sweep.

105 are procedures and 4 are module or program scopes. By directory: 86 in `src/`, 14 in `src/gpu/`, 7 in `examples/`, 2 in `test/`.

Largest files first:

- `src/SELF_Model.f90`, 10
- `src/SELF_BoundaryConditions.f90`, 7
- `src/SELF_ESAtmo3D_t.f90`, 6
- `src/SELF_ESAtmo2D_t.f90`, 6
- `src/SELF_Mesh_3D_t.f90`, 5
- `src/SELF_LinearEuler2D_t.f90`, 5
- `src/SELF_LinearEuler2D_PML_t.f90`, 5
- `src/SELF_advection_diffusion_3d_t.f90`, 5
- `src/SELF_advection_diffusion_2d_t.f90`, 5
- `src/SELF_LinearShallowWater2D_t.f90`, 4
- `src/SELF_LinearEuler3D_t.f90`, 4
- `src/SELF_ECAdvection3D_t.f90`, 4

Every change here needs the full gfortran, ifx, nvfortran and amdflang matrix before merge.

## Out of scope for this issue

Three CI defects found while building the checks, unrelated to style and worth their own issue:

- Every workflow except `amd-mi210-gpu-tests.yml` uses `paths-ignore: ['docs/']` with no `/**` glob, so the pattern never matches and documentation-only pull requests still trigger the full build matrix.
- `docs/self.md` has diverged from the root `self.md` FORD configuration; only the root one is used by `main-docs.yml`.
- `main-docs.yml` has no `pull_request` trigger, so the documentation build is never validated before merge.

## Notes

Do not disable a rule or raise a threshold in `.github/style-rules.json` to make a file pass. The thresholds are calibrated against `f3e1e57c`, the last commit before the first AI assisted contribution, and the reference tree satisfies them.

Contributor guide

Open the contributing guide

Research direction

Start by running .github/scripts/style_check.py and .github/scripts/docs_style_check.py, then read docs/Contributing/StyleGuide.md and choose one proposed sequence, beginning with F001 in the listed files. For F007, inspect each reported program unit and run the gfortran, ifx, nvfortran and amdflang matrix. Done means the selected rule violations are resolved without disabling rules, with documentation checks passing.

Written by the indexing model from the issue text.

Assessment

Tech stack
fortran, markdown, python
Domain
build-system, documentation, testing-qa
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.