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

🐛 BUG: astro check summary shows warnings as hints #914

Open
florian-lefebvre opened this issue Jul 24, 2024 · 6 comments
Open

🐛 BUG: astro check summary shows warnings as hints #914

florian-lefebvre opened this issue Jul 24, 2024 · 6 comments
Labels
- P2: nice to have Not breaking anything but nice to have (priority) ecosystem: upstream Issue is caused by a bug / missing feature upstream feat: check Issue in `astro-check` (scope)

Comments

@florian-lefebvre
Copy link
Member

Describe the Bug

When running astro check with warnings, the summary shows the same number as hints
image

Steps to Reproduce

  1. Get a warning and run astro check

(Don't use this reproduction as an example okay? Very bad)

@github-actions github-actions bot added the needs triage Issue needs to be triaged label Jul 24, 2024
@Princesseuh Princesseuh added - P2: nice to have Not breaking anything but nice to have (priority) feat: check Issue in `astro-check` (scope) and removed needs triage Issue needs to be triaged labels Jul 24, 2024
@Princesseuh
Copy link
Member

Not 100% sure if this is a printing or collecting issue, but:

The printing happens here:

console.info(
[
bold(`Result (${result.fileChecked} file${result.fileChecked === 1 ? '' : 's'}): `),
['error', 'warning', 'hint'].includes(minimumSeverity)
? bold(red(`${result.errors} ${result.errors === 1 ? 'error' : 'errors'}`))
: undefined,
['warning', 'hint'].includes(minimumSeverity)
? bold(yellow(`${result.warnings} ${result.warnings === 1 ? 'warning' : 'warnings'}`))
: undefined,
['hint'].includes(minimumSeverity)
? dim(`${result.hints} ${result.hints === 1 ? 'hint' : 'hints'}\n`)
: undefined,
]
.filter(Boolean)
.join(`\n${dim('-')} `)
);

and the collecting happens here:

result.errors += fileDiagnostics.filter(
(diag) => diag.severity === DiagnosticSeverity.Error
).length;
result.warnings += fileDiagnostics.filter(
(diag) => diag.severity === DiagnosticSeverity.Warning
).length;
result.hints += fileDiagnostics.filter(
(diag) => diag.severity === DiagnosticSeverity.Hint
).length;

Both seems correct to me at first glance, so not 100% sure where the issue is. Something to note is that TypeScript has a weird relationship with diagnostic severity, so that might be related too.

@Princesseuh
Copy link
Member

It's also possible for this mistake to be in Volar itself, see: https://github.com/volarjs/volar.js/blob/ea2e22ca17116b4274d661a014cff040c1d206cb/packages/kit/lib/createChecker.ts#L230

My recommendation here would be to match tsc.

@Princesseuh Princesseuh added the ecosystem: upstream Issue is caused by a bug / missing feature upstream label Aug 6, 2024
@fsmeier
Copy link

fsmeier commented Aug 30, 2024

+1

Any news on the issue? Seems like astro check is not that much usable with that. :/

I also tested astro check --minimumFailingSeverity hint --minimumSeverity hint but it seems to just continue.

@Princesseuh
Copy link
Member

I'm not sure I understand how this affects astro check's usability, this is mainly visual

@fsmeier
Copy link

fsmeier commented Aug 31, 2024

I think it is not just visual - with the option --minimumFailingSeverity warning or --minimumFailingSeverity hint it should have failed, but it did not in both cases. just with error (when i placed one)

@fsmeier
Copy link

fsmeier commented Aug 31, 2024

Hmm, maybe this is also a clue we could follow:

In the github action the option --minimumFailingSeverity warning seems to work and does not show the hints which are labeled "warning". Locally it will show them anyway.
In the ci i use ubuntu-latest. Locally I use node:20 which should be based on debian.
Screenshot 2024-08-31 at 10 25 35

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P2: nice to have Not breaking anything but nice to have (priority) ecosystem: upstream Issue is caused by a bug / missing feature upstream feat: check Issue in `astro-check` (scope)
Projects
None yet
Development

No branches or pull requests

3 participants