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

Fix unexpected error message in report() #7172

Closed
georges-g opened this issue Sep 6, 2023 · 8 comments · Fixed by #7474
Closed

Fix unexpected error message in report() #7172

georges-g opened this issue Sep 6, 2023 · 8 comments · Fixed by #7474
Labels
status: wip is being worked on by someone type: enhancement a new feature that isn't related to rules

Comments

@georges-g
Copy link

What minimal example or steps are needed to reproduce the bug?

Empty sass file

What minimal configuration is needed to reproduce the bug?

{
    "customSyntax": "postcss-sass",
    "extends": "stylelint-config-standard"
}

How did you run Stylelint?

npx stylelint -f verbose "**/*.sass"

Which Stylelint-related dependencies are you using?

  "devDependencies": {
    "postcss-sass": "^0.5.0",
    "stylelint": "^15.10.3",
    "stylelint-config-standard": "^34.0.0"
  }

What did you expect to happen?

No problem to report

What actually happened?

Stylelint crashed:

Error: You must pass either a node or a line number
    at report (/home/georges/data/tmp/dev/stylelint-empty-file/node_modules/stylelint/lib/utils/report.js:42:9)
    at /home/georges/data/tmp/dev/stylelint-empty-file/node_modules/stylelint/lib/rules/no-empty-source/index.js:32:3
    at /home/georges/data/tmp/dev/stylelint-empty-file/node_modules/stylelint/lib/lintPostcssResult.js:121:8
    at Array.map (<anonymous>)
    at lintPostcssResult (/home/georges/data/tmp/dev/stylelint-empty-file/node_modules/stylelint/lib/lintPostcssResult.js:111:18)
    at lintSource (/home/georges/data/tmp/dev/stylelint-empty-file/node_modules/stylelint/lib/lintSource.js:110:8)
    at async /home/georges/data/tmp/dev/stylelint-empty-file/node_modules/stylelint/lib/standalone.js:211:27
    at async Promise.all (index 1)
    at async standalone (/home/georges/data/tmp/dev/stylelint-empty-file/node_modules/stylelint/lib/standalone.js:254:22)

Do you have a proposal to fix the bug?

Stylelint fails to process an empty sass file.

Again, I am not sure if it comes from stylelint or postcss-sass plugin. It works properly with the postcss-scss plugin though.

Also, the error message is not very practical, it does not even specify which file made stylelint crash, even with the -f verbose option. Any way to improve debugging messages?

@ybiquitous
Copy link
Member

@georges-g Thanks again for the helpful report. I'm afraid that this also may be a bug of postcss-sass. See the following verification:

> require('postcss-scss').parse('').source
{
  input: Input {
    css: '',
    hasBOM: false,
    id: '<input css zG44F1>',
    [Symbol(fromOffsetCache)]: [ 0 ]
  },
  start: { column: 1, line: 1, offset: 0 },
  end: { column: 1, line: 1, offset: 0 }
}
> require('postcss-sass').parse('').source
{
  start: [ 0, 0 ],
  end: [ 0, 0 ],
  input: Input { css: '', hasBOM: false, id: '<input css Ef1dZl>' }
}

We can see the difference between the start and end properties' forms. This form is a Position type of PostCSS API. The type is required to have the column, line, and offset properties like postcss-scss.

@georges-g
Copy link
Author

@ybiquitous Thanks again for the useful reply.

I created an issue there.

There is still the issue of stylelint logging and the difficulty to identify which file made it crash. Should we discuss it here or do I create another issue/feature request?

@ybiquitous
Copy link
Member

Please feel free to open a Pull Request to improve the message. 👌🏼

@ybiquitous ybiquitous changed the title Fails to process and empty file Fix unexpected error message in report() Sep 6, 2023
@ybiquitous ybiquitous added status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule labels Sep 6, 2023
@georges-g
Copy link
Author

OK, I put this in my todo list, but anyone should feel free to take it. It's a pretty big project and it will take me some time to get to know the fundamentals of its codebase and contribute appropriately.

@ybiquitous
Copy link
Member

For example, does the following message satisfy your request?

Error: You must pass either a node or a line number (rule: no-empty-source, file: /path/to/foo.sass)

If so, we can easily change the error message raised in report().

@georges-g
Copy link
Author

That would be useful yes.

If you want to go deeper, I think you need several things:

  • If there is not one already, a logging library allowing different log levels
  • The option to choose the log level from the CLI
  • Some logging instructions at the right places with the right level

Then you will probably not need to customize every throw anymore. Easier said than done though, I know ^^

@ybiquitous
Copy link
Member

Honestly, such a big change is unneeded with this issue.

In addition, since we're mainly developing on the v16 branch instead of the main branch, we don't hope for a big change.

@georges-g
Copy link
Author

It might not be so needed for this specific issue, but it could be useful if you look at the bigger picture and consider various debugging issues.

IMO, you could leave this issue open for a while and see what other people think. It could wait for after v16 is released.

If you don't want that, you can go for the quickfix, up to you.

@ybiquitous ybiquitous added status: wip is being worked on by someone and removed status: ready to implement is ready to be worked on by someone labels Jan 20, 2024
@Mouvedia Mouvedia added type: enhancement a new feature that isn't related to rules and removed type: bug a problem with a feature or rule labels Jan 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: wip is being worked on by someone type: enhancement a new feature that isn't related to rules
Development

Successfully merging a pull request may close this issue.

3 participants