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

-D pedantic throws File is too long to fit on the screen (even if max-lines is disabled) #4322

Closed
BracketJohn opened this issue Jul 17, 2024 · 5 comments · Fixed by #5120
Closed
Assignees
Labels
C-bug Category - Bug

Comments

@BracketJohn
Copy link

Description

We are on oxlint@0.6.0. With the following .oxlintrc.json:

{
  "env": {
    "browser": true
  },
  "rules": {
    "max-lines": "off"
  }
}

And the following oxlint invocation:

> pnpm oxlint -c .oxlintrc.json --deny-warnings -D correctness -D perf -D suspicious -D pedantic
  ! File is too long to fit on the screen
  help: "server/trpc/routers/userTask/index.ts" seems like a minified file

Finished in 50ms on 766 files with 158 rules using 10 threads.
Found 0 warnings and 1 error.

The following invocation succeeds (note how I dropped -D pedantic):

pnpm oxlint -c .oxlintrc.json --deny-warnings -D correctness -D perf -D suspicious
Finished in 40ms on 766 files with 101 rules using 10 threads.
Found 0 warnings and 0 errors.

server/trpc/routers/userTask/index.ts is not a minified file. It's a tRPC router file with 556 LoC.

Expected behavior

Given the same .oxlintrc.json the invocation including -D pedantic returns:

> pnpm oxlint -c .oxlintrc.json --deny-warnings -D correctness -D perf -D suspicious -D pedantic

Finished in 50ms on 766 files with 158 rules using 10 threads.
Found 0 warnings and 0 errors.
@BracketJohn BracketJohn added the C-bug Category - Bug label Jul 17, 2024
@Boshen
Copy link
Member

Boshen commented Jul 17, 2024

For context, it throws this if column number exceeds 400.

if err_str.lines().any(|line| line.len() >= 400) {
let minified_diagnostic = Error::new(
OxcDiagnostic::warn("File is too long to fit on the screen")
.with_help(format!("{path:?} seems like a minified file")),
);
err_str = format!("{minified_diagnostic:?}");

@Boshen
Copy link
Member

Boshen commented Jul 17, 2024

An improvement should be print out the error message but don't print the code frame.

@BracketJohn
Copy link
Author

Just to add / clarify, as I'm unsure whether the change you proposed would address it: The invocation currently also fails with a non-zero status code, blocking CI from passing. This to me feels undesired. I lack experience to say whether this is actually undesired or correct.

@BracketJohn
Copy link
Author

Further info: I identified the line that is causing the error. It's the input-parameter line of this function-signature. It does not seem to exceed 400 column numbers (width as displayed by editor: 201):

export const getTasks = async (
  user: { email: string, role: UserRole },
  prisma: ExtendedPrismaClient,
  input: z.infer<typeof getTasksInputSchema> = { assignedToUsers: { in: [] }, assignedToRole: { in: [] }, isRelatedToOffer: 'is-related-or-unrelated', relatedOfferId: [], createdByEmail: { in: [] } },
) => {

}

Breaking this line as follows stops the File is too long to fit on the screen-error from being thrown:

export const getTasks = async (
  user: { email: string, role: UserRole },
  prisma: ExtendedPrismaClient,
  input: z.infer<typeof getTasksInputSchema> =
  { assignedToUsers:
      { in: [] }, assignedToRole: { in: [] }, isRelatedToOffer: 'is-related-or-unrelated', relatedOfferId: [], createdByEmail: { in: [] } },
) => {

}

And instead reveals the "true" error:

> oxlint -c .oxlintrc.json --deny-warnings -D correctness -D perf -D suspicious -D pedantic && eslint --max-warnings 0 .


  × eslint-plugin-unicorn(no-object-as-default-parameter): Do not use an object literal as default for parameter `input`.
    ╭─[server/trpc/routers/userTask/index.ts:33:3]
 32 │       input: z.infer<typeof getTasksInputSchema> =
 33 │ ╭─▶   { assignedToUsers:
 34 │ ╰─▶       { in: [] }, assignedToRole: { in: [] }, isRelatedToOffer: 'is-related-or-unrelated', relatedOfferId: [], createdByEmail: { in: [] } },
 35 │     ) => {
    ╰────

@Boshen Boshen self-assigned this Aug 18, 2024
Boshen added a commit that referenced this issue Aug 23, 2024
closes #4322

Setting the threshold to 1200 because graphical output may contain ansi escape codes and other decorations.
Boshen added a commit that referenced this issue Aug 23, 2024
)

closes #4322

Setting the threshold to 1200 because graphical output may contain ansi escape codes and other decorations.
@Boshen
Copy link
Member

Boshen commented Aug 23, 2024

Made some improvements in #5120

@Boshen Boshen closed this as completed Aug 23, 2024
Dunqing pushed a commit that referenced this issue Aug 23, 2024
)

closes #4322

Setting the threshold to 1200 because graphical output may contain ansi escape codes and other decorations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category - Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants