Skip to content

Enhance IndentationRule to take into account Label Indent#729

Merged
chrismair merged 1 commit into
CodeNarc:masterfrom
xmac11:spock-label-indent
Mar 12, 2023
Merged

Enhance IndentationRule to take into account Label Indent#729
chrismair merged 1 commit into
CodeNarc:masterfrom
xmac11:spock-label-indent

Conversation

@xmac11

@xmac11 xmac11 commented Mar 5, 2023 •
edited
Loading

Copy link
Copy Markdown

Fixes #712

Description

Currently, the following syntax is considered as a violation of IndentationRule

def "should be true"() {
    expect:
        1 == 1

This PR introduces an indentUnderLabel boolean property to make this configurable.
The default value is false in order to be backwards compatible.
When it is set to true, then code within the label should be indented with the number of spaces defined by the spacesPerIndentLevel property.

Additionally, the following change is made in order to allow labels without a text description.

private static boolean isSpockBlockLabel(Statement statement) {
    return (statement.statementLabel in SPOCK_BLOCKS &&
            statement instanceof ExpressionStatement &&
-           statement.expression.class == ConstantExpression &&
-           statement.expression.type.clazz == String
    )
}

How has this been tested

Unit test are added covering:

  • No violations, including multiple nested blocks
  • No violations for labels with and without a description
  • Violations when indentUnderLabel property is not respected

@xmac11 xmac11 force-pushed the spock-label-indent branch from a04aa19 to 217d8f2 Compare March 5, 2023 16:40
@xmac11 xmac11 mentioned this pull request Mar 5, 2023
Closed
@chrismair

chrismair commented Mar 8, 2023

Copy link
Copy Markdown
Contributor

Nice contribution. I like it.

Would it be acceptable and sufficient to just have a boolean that controls whether the statements under a label should be indented a single level or not?

The existing rule configuration property is spacesPerIndentLevel, and the rest of the rule logic is concerned with the indent level. Could we have this new behavior align with that? I'm hoping it is not a requirement that the statements under a label be indented for some number of spaces other than spacesPerIndentLevel.

@xmac11

xmac11 commented Mar 8, 2023

Copy link
Copy Markdown
Author

Good point @chrismair, I was also wondering the same.
The only reason I went with this approach is that Groovy Code Style exposes a separate property for Label indent.

Screenshot 2023-03-08 at 10 01 00 AM

But I suppose most users will be using the same number of spaces everywhere.
Please let me know what you prefer and I'll go with it.

@chrismair

chrismair commented Mar 9, 2023

Copy link
Copy Markdown
Contributor

I can think of good reasons for either way, but let's go with the boolean. I would suggest indentUnderLabel, but I'm open to alternatives -- trying to minimize ambiguity of what that property controls. It should default to false, of course.

@xmac11 xmac11 force-pushed the spock-label-indent branch from 217d8f2 to 7844f53 Compare March 10, 2023 09:04
@xmac11

xmac11 commented Mar 10, 2023

Copy link
Copy Markdown
Author

@chrismair done :)

@chrismair chrismair merged commit 939499c into CodeNarc:master Mar 12, 2023
@chrismair

chrismair commented Mar 12, 2023

Copy link
Copy Markdown
Contributor

Looks good. Thanks for the contribution.

@asteiner-psh asteiner-psh mentioned this pull request Jan 15, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Label indent - question

2 participants

@xmac11 @chrismair