Skip to content

Conversation

@krystophny
Copy link
Member

@krystophny krystophny commented Oct 12, 2025

User description

Summary

  • ensure the indentation pass realigns nested constructs even when the original source used inconsistent offsets
  • guard against zero indent sizes and expand the Fortran test suite with a reproducer covering single-line IF followed by a block IF
  • refresh example outputs to reflect the corrected indentation

Testing

  • python run_tests.py

PR Type

Bug fix, Enhancement


Description

  • Fix indentation drift after single-line IF statements

  • Add string concatenation whitespace formatting option

  • Realign mis-indented nested blocks properly

  • Expand test suite with indentation reproducer


Diagram Walkthrough

flowchart LR
  A["Indentation Logic"] --> B["Check Misalignment"]
  B --> C["Realign Nested Blocks"]
  D["Whitespace Formatting"] --> E["Add Concat Option"]
  E --> F["Format '//' Operator"]
  G["Test Suite"] --> H["Add IF Reproducer"]
Loading

File Walkthrough

Relevant files
Bug fix
__init__.py
Core indentation and whitespace formatting fixes                 

fprettify/init.py

  • Fix indentation logic to detect and realign mis-indented nested blocks
  • Add string concatenation whitespace option with '//' operator
    formatting
  • Extend whitespace arrays and argument parser for new concat option
  • Update whitespace dictionary building function
+23/-7   
Tests
example.f90
Updated example with corrected indentation                             

fortran_tests/after/example.f90

  • Correct indentation for nested do loops
  • Fix alignment of inner loop constructs
+3/-3     
example_swapcase.f90
Updated swapcase example with corrected indentation           

fortran_tests/after/example_swapcase.f90

  • Correct indentation for nested DO loops in uppercase version
  • Fix alignment of inner loop constructs
+3/-3     
indent_single_line_if.f90
Expected output for single-line IF test                                   

fortran_tests/after/indent_single_line_if.f90

  • New test file showing properly formatted single-line IF statements
  • Demonstrates correct indentation after single-line IF constructs
+15/-0   
indent_single_line_if.f90
Test input for single-line IF indentation bug                       

fortran_tests/before/indent_single_line_if.f90

  • New test input with mis-indented single-line IF statements
  • Contains inconsistent indentation to reproduce the bug
+15/-0   
expected_results
Update test result hashes                                                               

fortran_tests/test_results/expected_results

  • Update hash values for modified test files
  • Add hash for new single-line IF test case
+3/-2     
Documentation
README.md
Document string concatenation whitespace option                   

README.md

  • Document new --whitespace-concat option for string concatenation
    formatting
+3/-0     

@qodo-merge-pro
Copy link

qodo-merge-pro bot commented Oct 12, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-merge-pro
Copy link

qodo-merge-pro bot commented Oct 12, 2025

PR Code Suggestions ✨

Latest suggestions up to 8487922

CategorySuggestion                                                                                                                                    Impact
Possible issue
Stabilize concat spacing with offsets

In add_whitespace_charwise, correct the logic for formatting the // operator to
prevent an IndexError. The current implementation incorrectly mixes indices and
slicing between the original and formatted lines, which can fail when // appears
inside a string literal.

fprettify/init.py [1214-1223]

     def add_whitespace_charwise(line, spacey, scope_parser, format_decl, filename, line_nr):
-        ...
-+        # format string concatenation operator '//'
-+        if (char == '/' and line[pos:pos + 2] == "//" and (pos == 0 or line[pos - 1] != '/')
-+                and level == 0 and pos > end_of_delim):
-+            lhs = line_ftd[:pos + offset]
-+            rhs = line_ftd[pos + 2 + offset:]
-+            line_ftd = lhs.rstrip(' ') \
-+                    + ' ' * spacey[10] \
-+                    + "//" \
-+                    + ' ' * spacey[10] \
-+                    + rhs.lstrip(' ')
+        line_ftd = line
+        pos_eq = []
+        end_of_delim = -1
+        level = 0
+        for pos, char in CharFilter(line):
+            offset = len(line_ftd) - len(line)
+            ...
+            # format string concatenation operator '//'
+            if (char == '/' and line[pos:pos + 2] == "//"
+                    and (pos == 0 or line[pos - 1] != '/')
+                    and level == 0 and pos > end_of_delim):
+                insert_pos = pos + offset
+                if insert_pos + 2 <= len(line_ftd):
+                    lhs = line_ftd[:insert_pos]
+                    rhs = line_ftd[insert_pos + 2:]
+                else:
+                    lhs = line_ftd[:insert_pos]
+                    rhs = ""
+                line_ftd = lhs.rstrip(' ') \
+                        + ' ' * spacey[10] \
+                        + "//" \
+                        + ' ' * spacey[10] \
+                        + rhs.lstrip(' ')
+            ...

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a critical bug where string slicing on line with an index from line_ftd can cause an IndexError if the line contains characters like // inside string literals or comments, which this PR aims to format.

Medium
Incremental [*]
Harmonize concat whitespace defaults

Harmonize the default whitespace settings for the new 'concat' operator with
other similar operators across different whitespace levels (1-3) to ensure
consistent formatting behavior.

fprettify/init.py [1037-1060]

 mapping = {
-        'comma': 0,           # 0: comma, semicolon
-        'assignments': 1,     # 1: assignment operators
-        'relational': 2,      # 2: relational operators
-        'logical': 3,         # 3: logical operators
-        'plusminus': 4,       # 4: arithm. operators plus and minus
-        'multdiv': 5,         # 5: arithm. operators multiply and divide
-        'print': 6,           # 6: print / read statements
-        'type': 7,            # 7: select type components
-        'intrinsics': 8,      # 8: intrinsics
-        'decl': 9,            # 9: declarations
-        'concat': 10          # 10: string concatenation
+        'comma': 0,
+        'assignments': 1,
+        'relational': 2,
+        'logical': 3,
+        'plusminus': 4,
+        'multdiv': 5,
+        'print': 6,
+        'type': 7,
+        'intrinsics': 8,
+        'decl': 9,
+        'concat': 10
         }
-...
+if whitespace == 0:
+    spacey = [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
 elif whitespace == 1:
-    spacey = [1, 1, 1, 1, 0, 0, 1, 0, 1, 1, 0]
+    # enable concat similar to assignments formatting at level 1
+    spacey = [1, 1, 1, 1, 0, 0, 1, 0, 1, 1, 1]
 elif whitespace == 2:
-    spacey = [1, 1, 1, 1, 1, 0, 1, 0, 1, 1, 0]
+    spacey = [1, 1, 1, 1, 1, 0, 1, 0, 1, 1, 1]
 elif whitespace == 3:
-    spacey = [1, 1, 1, 1, 1, 1, 1, 0, 1, 1, 0]
+    spacey = [1, 1, 1, 1, 1, 1, 1, 0, 1, 1, 1]
 elif whitespace == 4:
     spacey = [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1]
+else:
+    raise NotImplementedError("unknown value for whitespace")

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 5

__

Why: The suggestion correctly points out that the new 'concat' operator has different default spacing behavior than other operators, which could be surprising. Improving this would enhance usability and consistency.

Low
  • Update

Previous suggestions

Suggestions up to commit 01e2850
CategorySuggestion                                                                                                                                    Impact
Incremental [*]
Avoid formatting inside strings/comments

Prevent formatting of the string concatenation operator // when it appears
inside string literals or comments. Use existing context-tracking variables like
level and what_del_open to apply the formatting logic only to actual code.

fprettify/init.py [1214-1223]

 # format string concatenation operator '//'
-if char == '/' and line[pos:pos + 2] == "//" and (pos == 0 or line[pos - 1] != '/'):
+if (
+    char == '/'
+    and line[pos:pos + 2] == "//"
+    and (pos == 0 or line[pos - 1] != '/')
+    # avoid inside delimiters (quotes, comments, etc.)
+    and level == 0
+    and what_del_open is None
+):
     lhs = line_ftd[:pos + offset]
     rhs = line_ftd[pos + 2 + offset:]
     line_ftd = lhs.rstrip(' ') \
             + ' ' * spacey[10] \
             + "//" \
             + ' ' * spacey[10] \
             + rhs.lstrip(' ')
     line_ftd = line_ftd.rstrip(' ')
Suggestion importance[1-10]: 8

__

Why: This suggestion correctly identifies a critical bug in the new feature where formatting the // operator would incorrectly modify string literals containing //, potentially breaking the code. The proposed fix correctly uses existing context-aware variables to prevent this.

Medium
Guard against invalid indent size

Add a defensive guard to handle cases where indent_size is zero or negative.
This prevents potential crashes or misformatting if an invalid indent value is
provided.

fprettify/init.py [878-884]

 # don't impose indentation for blocked do/if constructs:
 if (IF_RE.search(f_line) or DO_RE.search(f_line)):
-    indent_misaligned = indent_size > 0 and offset % indent_size != 0
+    safe_indent_size = indent_size if indent_size and indent_size > 0 else 1
+    indent_misaligned = (offset % safe_indent_size) != 0
     if (prev_offset != offset or strict_indent or indent_misaligned):
-        indents[-1] = indent_size
+        indents[-1] = safe_indent_size
 else:
-    indents[-1] = indent_size
+    indents[-1] = indent_size if indent_size and indent_size > 0 else 1
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies that an indent_size of 0 is possible and could cause issues, although the PR author already guarded the new code path. It proposes a valid defensive improvement, enhancing robustness against misconfiguration.

Low
Suggestions up to commit 85c3890
CategorySuggestion                                                                                                                                    Impact
Possible issue
Remove premature trailing whitespace stripping

Remove the line_ftd = line_ftd.rstrip(' ') line from the string concatenation
formatting logic to prevent incorrect removal of trailing spaces.

fprettify/init.py [1214-1223]

 # format string concatenation operator '//'
 if char == '/' and line[pos:pos + 2] == "//" and (pos == 0 or line[pos - 1] != '/'):
     lhs = line_ftd[:pos + offset]
     rhs = line_ftd[pos + 2 + offset:]
     line_ftd = lhs.rstrip(' ') \
             + ' ' * spacey[10] \
             + "//" \
             + ' ' * spacey[10] \
             + rhs.lstrip(' ')
-    line_ftd = line_ftd.rstrip(' ')
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that line_ftd.rstrip(' ') can cause incorrect formatting by prematurely removing a trailing space after the // operator, which might be needed before a line break.

Medium

Address Qodo review comments:
- Add level and end_of_delim checks to avoid formatting // inside strings/comments
- Remove unsafe trailing rstrip that could strip string literal whitespace
- Enable concat spacing at whitespace level 4
- Add comprehensive test coverage for concat operator formatting
Copy link
Collaborator

@zandivx zandivx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't reproduce the problem before this change, so please provide an example of the actual problem.

DO j = 1, 2
WRITE (*, *) test_function(m, r, k, l) + i
END DO
DO j = 1, 2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated, remove

@krystophny krystophny merged commit 2b24878 into master Oct 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants