prettier / plugin-pug

Prettier Pug Plugin
https://prettier.github.io/plugin-pug
MIT License
199 stars 44 forks source link

Bug: pugClassNotation: 'attribute' outputs junk if class literals are after attributes #508

Closed MrHaila closed 4 weeks ago

MrHaila commented 2 months ago

Plugin Version

3.1.0

Prettier Version

3.3.3

Which frameworks are affected?

Node Version

20.17.0

Which operating systems have you used?

Prettier config

{
  pugClassNotation: 'attribute'
}

Input

div.foo
div(attribute="value").foo
div(attribute="value" class="bar").foo
div(attribute="value" class="bar").foo.baz
div(
  attribute="value"
  class="bar"
).foo.baz
div.foo(attribute="value" class="bar").baz
div: span.foo
div: span(class="foo")
div.foo: span.bar(class="baz").foo
div.foo(class="bar").baz: h1.foo(class="bar").baz Let's really #[span.foo(class="bar").baz get nasty]!

Output or Error

(class="foo")
div(attribute="value")div(class="foo")
div(attribute="value", class="bar")div(class="foo")
div(attribute="value", class="bar")div(class="foo baz")
div(attribute="value", class="bar")div(class="foo baz")
(attribute="value", class="foo bar")div(class="baz")
div: span(class="foo") // <- this line was correct!
div: span(class="foo") // <- this line was correct!
div: span(class="foo")
: span(class="foo bar baz")div(class="foo")
(class="foo bar")div: h1(class="baz foo bar")div(class="baz") Let's really #[span(class="foo bar")div(class="baz") get nasty]!

Expected Output

div(class="foo")
div(attribute="value", class="foo")
div(attribute="value")
div(attribute="value", class="bar foo")
div(attribute="value", class="bar foo baz")
div(attribute="value", class="bar foo baz")
div(attribute="value", class="bar foo baz")
div: span(class="foo")
div: span(class="foo")
div(class="foo"): span(class="bar baz foo")
div(class="foo bar baz"): h1(class="foo bar baz") Let's really #[span(class="foo bar baz") get nasty]!

Additional Context

My workaround for this issue was to first use the option that refactors all class literals to the beginning and only then enable this class attribute option.

This is easy to repro by expanding the pugClassNotation test with the above code.

The offending code is easy to spot at the beginning of the private class function in printer.ts, but it's not immediately clear to me why that code works in the first place (do multiple chained attributes get merged by some follow-up logic?) or how to fix it properly. I'll open a PR if I can puzzle it out 🤷‍♂️

My verbosely commented version of the code that needs fixing:

    // Special handling for class literals when using the attribute notation.
    if (this.options.pugClassNotation === 'attribute') {
      // Add the class to the list of class literals to be converted to attributes.
      this.classLiteralsToBeConvertedToAttributes.push(token.val);

      // TODO: This can print junk (bug) if the class literal is after attributes. Investigate.
      if (
        this.previousToken?.type !== 'tag' &&
        this.previousToken?.type !== 'class'
      ) {
        this.result += `${this.computedIndent}div`;
      }

      // If this is the last class token (i.e. we are done with the literals)...
      // TODO: This probably does not handle inline elements chained with : correctly. Investigate.
      if (
        this.nextToken &&
        ['text', 'newline', 'indent', 'outdent', 'eos'].includes(
          this.nextToken.type,
        )
      ) {
        // Take a copy of the class literals to be converted to attributes and clear the list.
        const classes: string[] =
          this.classLiteralsToBeConvertedToAttributes.splice(
            0,
            this.classLiteralsToBeConvertedToAttributes.length,
          );

        // Print all the class literals as one class attribute.
        // TODO: What happens if the class attribute is already present? Investigate.
        this.result += `(class=${this.quoteString(classes.join(' '))})`;
        if (this.nextToken.type === 'text') {
          this.result += ' ';
        }
      }

      // Done processing the class token. Return early.
      return;
    }
Shinigami92 commented 2 months ago

Is this a duplicate of https://github.com/prettier/plugin-pug/issues/167?

MrHaila commented 2 months ago

I believe these are bugs in #203 that partially implemented #167 so not a direct duplicate.

Fully completing #167 would mean adding a new option. This issue highlights bugs in the already existing option that were not covered by tests.

Shinigami92 commented 2 months ago

You could either extend the tests in https://github.com/prettier/plugin-pug/tree/main/tests/options/pugClassNotation or add a new test in https://github.com/prettier/plugin-pug/tree/main/tests/issues in a new folder issue-508