Skip to content
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.

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

[WIP] [Fix: #11462] Fix over-indentation in Layout/FirstElementHashIndentation #11463

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sambostock
Copy link
Contributor

This investigates fixing #11462. So far, it adds an autocorrection test. I'm still figuring out the details of the autocorrector logic, which I am very much unfamiliar with (cc. @dvandersluis ).

Starting with the example:

def example
  { key1:
    { key2:
      { key3:
	{ key4:
	  { key5:
	    :value
	  }
	}
      }
    }
  }
end

What seems to be happening is that a line break is inserted between { and keyN:. Layout/TrailingWhitespace handles getting rid of the space remaining after {, but everything goes wrong in Layout/FirstHashElementIndentation.

def example
  {
key1:
    {
key2:
      {
key3:
	{
key4:
	  {
key5:
	    :value
	  }
	}
      }
    }
  }
end

It finds key1: in column 0, computes a column delta of 4, which it applies to the key and value lines, even though the value lines don't have the same indentation.

def example
  {
    key1:
        {
    key2:
          {
    key3:
    	{
    key4:
    	  {
    key5:
    	    :value
    	  }
    	}
          }
        }
  }
end

This repeats for key2:, which is not indented as much as it should be relative to its opening {, so the entire thing is indented by the column delta.

def example
  {
    key1:
        {
          key2:
                {
          key3:
          	{
          key4:
          	  {
          key5:
          	    :value
          	  }
          	}
                }
        }
  }
end

This continues to repeat for key3:, key4:, and key5:, respectively:

def example
  {
    key1:
        {
          key2:
                {
                  key3:
                  	{
                  key4:
                  	  {
                  key5:
                  	    :value
                  	  }
                  	}
                }
        }
  }
end
def example
  {
    key1:
        {
          key2:
                {
                  key3:
                  	{
                          key4:
                          	  {
                          key5:
                          	    :value
                          	  }
                  	}
                }
        }
  }
end
def example
  {
    key1:
        {
          key2:
                {
                  key3:
                  	{
                          key4:
                          	  {
                                    key5:
                                    	    :value
                          	  }
                  	}
                }
        }
  }
end

Every level is incorrectly corrected once for each of it's parents, and finally for itself, compounding the error.

Replace this text with a summary of the changes in your PR.
The more detailed you are, the better.


Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

Comment on lines 2785 to 2799
create_file('.rubocop.yml', <<~YAML)
AllCops:
DisabledByDefault: true
Style/FrozenStringLiteralComment:
Enabled: false

Layout/FirstHashElementIndentation:
Enabled: true
Layout/FirstHashElementLineBreak:
Enabled: true
Layout/TrailingWhitespace:
Enabled: true
YAML

status = cli.run(%w[--autocorrect-all])
Copy link
Member

Choose a reason for hiding this comment

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

You should probably just specify (with --only, see line 2666 for an example) which cops you want to run for this test, and then you wouldn't need to explicitly enable/disable cops in the yaml file. It should work fine without explicitly creating a .rubocop.yml file.

Otherwise, test looks alright to me!

```ruby
def example
  { key1:
    { key2:
      { key3:
	{ key4:
	  { key5:
	    :value
	  }
	}
      }
    }
  }
end
```

What seems to be happening is that a line break is inserted between `{`
and `keyN:`. `Layout/TrailingWhitespace` handles getting rid of the
space remaining after `{`, but everything goes wrong in
`Layout/FirstHashElementIndentation`.

```ruby
def example
  {
key1:
    {
key2:
      {
key3:
	{
key4:
	  {
key5:
	    :value
	  }
	}
      }
    }
  }
end
```

It finds `key1:` in column 0, computes a column delta of 4, which it
applies to the key and value lines, even though the value lines don't
have the same indentation.

```ruby
def example
  {
    key1:
        {
    key2:
          {
    key3:
    	{
    key4:
    	  {
    key5:
    	    :value
    	  }
    	}
          }
        }
  }
end
```

This repeats for `key2:`, which is not indented as much as it should be
relative to its opening `{`, so the entire thing is indented by the
column delta.

```ruby
def example
  {
    key1:
        {
          key2:
                {
          key3:
          	{
          key4:
          	  {
          key5:
          	    :value
          	  }
          	}
                }
        }
  }
end
```

This continues to repeat for `key3:`, `key4:`, and `key5:`,
respectively:

```ruby
def example
  {
    key1:
        {
          key2:
                {
                  key3:
                  	{
                  key4:
                  	  {
                  key5:
                  	    :value
                  	  }
                  	}
                }
        }
  }
end
```

```ruby
def example
  {
    key1:
        {
          key2:
                {
                  key3:
                  	{
                          key4:
                          	  {
                          key5:
                          	    :value
                          	  }
                  	}
                }
        }
  }
end
```

```ruby
def example
  {
    key1:
        {
          key2:
                {
                  key3:
                  	{
                          key4:
                          	  {
                                    key5:
                                    	    :value
                          	  }
                  	}
                }
        }
  }
end
```

Every level is incorrectly corrected once for each of it's parents, and
finally for itself, compounding the error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants