Skip to content

Compensate depth for compound blocks inside HashIf content #134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
jvsqzj wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Compensate depth for compound blocks inside HashIf content #134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cb229d5
Compensate depth for compound blocks inside HashIf content
jvsqzj Aug 22, 2025
0452e30
Add test for nested HashIf code blocks
jvsqzj Dec 19, 2025
e708286
Pass parent_start_col through AuxParams for required cases
jvsqzj Jan 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/analysis/parsing/statement.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ impl TreeElement for CompoundContent {
fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, aux: AuxParams) {
rules.sp_brace.check(SpBracesArgs::from_compound(self), acc);
rules.indent_code_block.check(IndentCodeBlockArgs::from_compound_content(self, aux.depth), acc);
rules.indent_closing_brace.check(IndentClosingBraceArgs::from_compound_content(self, aux.depth), acc);
rules.indent_closing_brace.check(IndentClosingBraceArgs::from_compound_content(self, aux.parent_statement_start_col), acc);
}
fn should_increment_depth(&self) -> bool {
true
Expand Down Expand Up @@ -439,6 +439,9 @@ impl TreeElement for IfContent {
&self.truebranch,
&self.elsebranch)
}
fn should_save_outer_start_col(&self) -> bool {
true
}
fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, _aux: AuxParams) {
rules.nsp_inparen.check(NspInparenArgs::from_if(self), acc);
rules.indent_paren_expr.check(IndentParenExprArgs::from_if(self), acc);
Expand Down Expand Up @@ -1122,7 +1125,7 @@ impl TreeElement for SwitchContent {
fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>,
rules: &CurrentRules, aux: AuxParams)
{
rules.indent_closing_brace.check(IndentClosingBraceArgs::from_switch_content(self, aux.depth), acc);
rules.indent_closing_brace.check(IndentClosingBraceArgs::from_switch_content(self, self.switchtok.range().col_start.0), acc);
rules.indent_paren_expr.check(IndentParenExprArgs::from_switch(self), acc);
}
}
Expand Down
34 changes: 33 additions & 1 deletion src/analysis/parsing/structure.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ use crate::analysis::FileSpec;
use crate::span::Range;
use crate::vfs::TextFile;

use super::parser::Token;

fn object_contexts(context: &ParseContext)
-> (ParseContext, ParseContext) {
fn understands_semi(token: TokenKind) -> bool {
Expand Down Expand Up @@ -239,6 +241,9 @@ impl TreeElement for MethodContent {
}
errors
}
fn should_save_outer_start_col(&self) -> bool {
true
}
fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, aux: AuxParams) {
rules.nsp_funpar.check(NspFunparArgs::from_method(self), acc);
rules.nsp_inparen.check(NspInparenArgs::from_method(self), acc);
Expand Down Expand Up @@ -731,7 +736,7 @@ impl TreeElement for ObjectStatementsContent {
fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, aux: AuxParams) {
rules.sp_brace.check(SpBracesArgs::from_obj_stmts(self), acc);
rules.indent_code_block.check(IndentCodeBlockArgs::from_obj_stmts_content(self, aux.depth), acc);
rules.indent_closing_brace.check(IndentClosingBraceArgs::from_obj_stmts_content(self, aux.depth), acc);
rules.indent_closing_brace.check(IndentClosingBraceArgs::from_obj_stmts_content(self, aux.parent_statement_start_col), acc);
}
fn should_increment_depth(&self) -> bool {
matches!(self, ObjectStatementsContent::List(lbrace, list, rbrace)
Expand Down Expand Up @@ -1061,6 +1066,9 @@ impl TreeElement for RegisterContent {
}
}
fn references<'a>(&self, _accumulator: &mut Vec<Reference>, _file: FileSpec<'a>) {}
fn should_save_outer_start_col(&self) -> bool {
true
}
}

impl Parse<DMLObjectContent> for RegisterContent {
Expand Down Expand Up @@ -1130,6 +1138,9 @@ impl TreeElement for FieldContent {
}
}
fn references<'a>(&self, _accumulator: &mut Vec<Reference>, _file: FileSpec<'a>) {}
fn should_save_outer_start_col(&self) -> bool {
true
}
}

impl Parse<DMLObjectContent> for FieldContent {
Expand Down Expand Up @@ -1384,6 +1395,19 @@ impl TreeElement for HashIfContent {
}
errors
}
fn style_check(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, mut aux: AuxParams) {
self.evaluate_rules(acc, rules, aux);
self.cond.style_check(acc, rules, aux);
// if depth=0 for this HashIf statement
// then pass 0xffff_ffff interpreted as -1
// to child block, so no indentation is applied
if aux.depth == 0 {
aux.depth = 0xffff_ffff;
}

self.truestatements.style_check(acc, rules, aux);
self.elsebranch.style_check(acc, rules, aux);
}
}

impl Parse<DMLObjectContent> for HashIfContent {
Expand Down Expand Up @@ -1602,6 +1626,9 @@ impl TreeElement for TypedefContent {
fn post_parse_sanity(&self, _file: &TextFile) -> Vec<LocalDMLError> {
self.decl.ensure_named()
}
fn should_save_outer_start_col(&self) -> bool {
true
}
}

// Special case, to distinguish between extern cdecl and extern typedef, the
Expand Down Expand Up @@ -1940,6 +1967,11 @@ impl TreeElement for DMLObjectContent {
fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, aux: AuxParams) {
rules.indent_continuation_line.check(acc, IndentContinuationLineArgs::from_dml_object_content(self, aux.depth));
}
fn save_outer_start_col(&self, aux: &mut AuxParams) {
if let Some(token) = self.tokens().first() {
aux.parent_statement_start_col = token.range.col_start.0;
}
}
}

pub type DMLObject = AstObject<DMLObjectContent>;
Expand Down
12 changes: 10 additions & 2 deletions src/analysis/parsing/tree.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,17 +107,25 @@ pub trait TreeElement {
}

fn style_check(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, mut aux: AuxParams) {
if self.should_save_outer_start_col() {
self.save_outer_start_col(&mut aux);
}
if self.should_increment_depth() {
aux.depth += 1;
aux.depth = aux.depth.wrapping_add(1);
}
Comment on lines 113 to 115
Copy link
Contributor Author

@jvsqzj jvsqzj Dec 19, 2025

Choose a reason for hiding this comment

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

@JonatanWaern we'd like to hear your comments on this solution.
We are using this u32 value of 0xFFFF_FFFF to signal the child code block of a top-level #if/#else block it should actually use a zero-level indentation (ie remain at the top level).

We did not want to change the type of aux.depth to i32 as there will never be a negative indentation, so IMO it would add far more possibilities of undefined behavior to allow for negative values here than to just interpret the biggest possible value as a boundary.

The wrapping_add takes care of taking the depth to the corresponding value accordingly; either 0 for top level, or n+1 for any other nested #if/#else

Copy link
Contributor

@JonatanWaern JonatanWaern Jan 12, 2026

Choose a reason for hiding this comment

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

I dislike in-type magic values in rust on principle, and AFAIK it's best practice to avoid. In particular setting it to a magic max value, and relying on a wrapping add seems hacky to me.

Instead, it may be that indentation is sufficiently complicated that we should introduce a dedicated structure/enum to support it? (which could support incrementation and hard-set overrides) I have some ideas for this, so I may try my hand at it later (at the moment I need to get the builds to work again)

Copy link
Contributor

@JonatanWaern JonatanWaern Jan 12, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I have prepared a draft for what such a handler could look like in #185, you could rebase on it to see if it fits for what you need

self.evaluate_rules(acc, rules, aux);
for sub in self.subs() {
sub.style_check(acc, rules, aux);
}
}
fn should_increment_depth(&self) -> bool {false} // default don't increment
fn should_save_outer_start_col(&self) -> bool {false} // default don't save
fn evaluate_rules(&self, _acc: &mut Vec<DMLStyleError>, _rules: &CurrentRules, _aux: AuxParams) {} // default NOOP
}
fn save_outer_start_col(&self, aux: &mut AuxParams) {
if let Some(token) = self.tokens().first() {
aux.parent_statement_start_col = token.range.col_start.0;
}
}}

impl <T: ?Sized + TreeElement> ReferenceContainer for T {
fn collect_references<'a>(&self,
Expand Down
9 changes: 6 additions & 3 deletions src/analysis/parsing/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl TreeElement for StructTypeContent {
}
fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, aux: AuxParams) {
rules.indent_code_block.check(IndentCodeBlockArgs::from_struct_type_content(self, aux.depth), acc);
rules.indent_closing_brace.check(IndentClosingBraceArgs::from_struct_type_content(self, aux.depth), acc);
rules.indent_closing_brace.check(IndentClosingBraceArgs::from_struct_type_content(self, self.structtok.range().col_start.0), acc);
rules.sp_brace.check(SpBracesArgs::from_struct_type_content(self), acc);
}
fn should_increment_depth(&self) -> bool {
Expand Down Expand Up @@ -139,7 +139,7 @@ impl TreeElement for LayoutContent {
}
fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, aux: AuxParams) {
rules.indent_code_block.check(IndentCodeBlockArgs::from_layout_content(self, aux.depth), acc);
rules.indent_closing_brace.check(IndentClosingBraceArgs::from_layout_content(self, aux.depth), acc);
rules.indent_closing_brace.check(IndentClosingBraceArgs::from_layout_content(self, self.layout.range().col_start.0), acc);
rules.sp_brace.check(SpBracesArgs::from_layout_content(self), acc);
}
fn should_increment_depth(&self) -> bool {
Expand Down Expand Up @@ -317,11 +317,14 @@ impl TreeElement for BitfieldsContent {
fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, aux: AuxParams) {
rules.sp_brace.check(SpBracesArgs::from_bitfields_content(self), acc);
rules.indent_code_block.check(IndentCodeBlockArgs::from_bitfields_content(self, aux.depth), acc);
rules.indent_closing_brace.check(IndentClosingBraceArgs::from_bitfields_content(self, aux.depth), acc);
rules.indent_closing_brace.check(IndentClosingBraceArgs::from_bitfields_content(self, self.bitfields.range().col_start.0), acc);
}
fn should_increment_depth(&self) -> bool {
true
}
fn should_save_outer_start_col(&self) -> bool {
true
}
}

impl Parse<BaseTypeContent> for BitfieldsContent {
Expand Down
4 changes: 3 additions & 1 deletion src/lint/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ impl LinterAnalysis {
fn begin_style_check(ast: TopAst, file: &str, rules: &CurrentRules) -> Result<Vec<DMLStyleError>, Error> {
let (mut invalid_lint_annot, lint_annot) = obtain_lint_annotations(file);
let mut linting_errors: Vec<DMLStyleError> = vec![];
ast.style_check(&mut linting_errors, rules, AuxParams { depth: 0 });
ast.style_check(&mut linting_errors, rules, AuxParams { depth: 0, parent_statement_start_col: 0 });

// Per line checks
let lines: Vec<&str> = file.lines().collect();
Expand Down Expand Up @@ -438,6 +438,8 @@ pub struct AuxParams {
// Individual nodes update depth to affect level of their
// nested TreeElements. See more in src/lint/README.md
pub depth: u32,
pub parent_statement_start_col: u32,
// pub get_parent_start_col: fn(T: dyn TreeElement) -> u32,
}

pub mod rules;
Expand Down
16 changes: 8 additions & 8 deletions src/lint/rules/indentation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ pub struct IndentClosingBraceArgs {
impl IndentClosingBraceArgs {
pub fn from_compound_content(node: &CompoundContent, depth: u32) -> Option<IndentClosingBraceArgs> {
Some(IndentClosingBraceArgs {
expected_depth: depth.saturating_sub(1),
expected_depth: depth,
lbrace: node.lbrace.range(),
last_member: node.statements.last()?.range(),
rbrace: node.rbrace.range(),
Expand All @@ -296,7 +296,7 @@ impl IndentClosingBraceArgs {
pub fn from_obj_stmts_content(node: &ObjectStatementsContent, depth: u32) -> Option<IndentClosingBraceArgs> {
if let ObjectStatementsContent::List(lbrace, stmnts, rbrace) = node {
Some(IndentClosingBraceArgs {
expected_depth: depth.saturating_sub(1),
expected_depth: depth,
lbrace: lbrace.range(),
last_member: stmnts.last()?.range(),
rbrace: rbrace.range(),
Expand All @@ -319,7 +319,7 @@ impl IndentClosingBraceArgs {

pub fn from_struct_type_content(node: &StructTypeContent, depth: u32) -> Option<IndentClosingBraceArgs> {
Some(IndentClosingBraceArgs {
expected_depth: depth.saturating_sub(1),
expected_depth: depth,
lbrace: node.lbrace.range(),
last_member: node.members.last()?.range(),
rbrace: node.rbrace.range(),
Expand All @@ -328,7 +328,7 @@ impl IndentClosingBraceArgs {

pub fn from_layout_content(node: &LayoutContent, depth: u32) -> Option<IndentClosingBraceArgs> {
Some(IndentClosingBraceArgs {
expected_depth: depth.saturating_sub(1),
expected_depth: depth,
lbrace: node.lbrace.range(),
last_member: node.fields.last()?.range(),
rbrace: node.rbrace.range(),
Expand All @@ -337,7 +337,7 @@ impl IndentClosingBraceArgs {

pub fn from_bitfields_content(node: &BitfieldsContent, depth: u32) -> Option<IndentClosingBraceArgs> {
Some(IndentClosingBraceArgs {
expected_depth: depth.saturating_sub(1),
expected_depth: depth,
lbrace: node.lbrace.range(),
last_member: node.fields.last()?.range(),
rbrace: node.rbrace.range(),
Expand Down Expand Up @@ -377,9 +377,9 @@ impl IndentClosingBraceRule {
return;
}

let rbrace_on_same_ind_level_than_switchtok:bool = args.rbrace.col_start.0
== args.expected_depth * self.indentation_spaces;
if !rbrace_on_same_ind_level_than_switchtok {
let rbrace_on_same_col_as_outer_statement_start:bool = args.rbrace.col_start.0
== args.expected_depth;
if !rbrace_on_same_col_as_outer_statement_start {
acc.push(self.create_err(args.rbrace));
}
}
Expand Down
97 changes: 97 additions & 0 deletions src/lint/rules/tests/indentation/code_block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,3 +298,100 @@ fn template_incorrect() {
);
assert_snippet(TEMPLATE_INCORRECT, expected_errors, &rules);
}

static TOPLEVEL_HASHIF_CORRECT: &str = "
constant FEATURE_SUPPORTED = false;

#if (!FEATURE_SUPPORTED) {
attribute target_sb_port is uint64_attr {}
} #else {
attribute target_sb_fid is uint64_attr {}
}

#if (!FEATURE_SUPPORTED) {
method signal_feature() {
return 1;
}
} #else {
method signal_feature() { return 0; }
}
";

#[test]
fn toplevel_hashif_correct() {
let rules = set_up();
assert_snippet(TOPLEVEL_HASHIF_CORRECT, vec![], &rules);
}

static TOPLEVEL_HASHIF_INCORRECT: &str = "
constant FEATURE_SUPPORTED = false;

#if (!FEATURE_SUPPORTED) {
attribute target_sb_port is uint64_attr {}
} #else {
attribute target_sb_fid is uint64_attr {}
}

#if (!FEATURE_SUPPORTED) {
method signal_feature() {
return 1;
}
} #else {
method signal_feature() { return 0; }
}
";

#[test]
fn toplevel_hashif_incorrect() {
let rules = set_up();
let expected_errors = define_expected_errors!(
RuleType::IN3,
(4, 4, 4, 46),
(6, 6, 4, 45),
(10, 12, 4, 5),
(11, 11, 8, 17),
(14, 14, 4, 41),
);
assert_snippet(TOPLEVEL_HASHIF_INCORRECT, expected_errors, &rules);
}

static NESTED_HASHIF_CORRECT: &str = "
constant FEATURE_SUPPORTED = false;

method signal_feature() {
#if (!FEATURE_SUPPORTED) {
return 1;
} #else {
return 0;
}
}
";

#[test]
fn nested_hashif_correct() {
let rules = set_up();
assert_snippet(NESTED_HASHIF_CORRECT, vec![], &rules);
}

static NESTED_HASHIF_INCORRECT: &str = "
constant FEATURE_SUPPORTED = false;

method signal_feature() {
#if (!FEATURE_SUPPORTED) {
return 1;
} #else {
return 0;
}
}
";

#[test]
fn nested_hashif_incorrect() {
let rules = set_up();
let expected_errors = define_expected_errors!(
RuleType::IN3,
(5, 5, 0, 9),
(7, 7, 12, 21),
);
assert_snippet(NESTED_HASHIF_INCORRECT, expected_errors, &rules);
}
Loading