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

πŸ’… useFilenamingConvention false positives on number-prefixed and bracket-wrapped files #3952

Closed
1 task done
sleewoo opened this issue Sep 17, 2024 · 13 comments Β· Fixed by #3957
Closed
1 task done
Assignees

Comments

@sleewoo
Copy link

sleewoo commented Sep 17, 2024

Environment information

➜ biome rage --linter
CLI:
  Version:                      1.9.1
  Color support:                true

Platform:
  CPU Architecture:             x86_64
  OS:                           linux

Environment:
  BIOME_LOG_PATH:               unset
  BIOME_LOG_PREFIX_NAME:        unset
  BIOME_CONFIG_PATH:            unset
  NO_COLOR:                     unset
  TERM:                         "xterm-256color"
  JS_RUNTIME_VERSION:           "v22.6.0"
  JS_RUNTIME_NAME:              "node"
  NODE_PACKAGE_MANAGER:         unset

Biome Configuration:
  Status:                       Loaded successfully
  Formatter disabled:           false
  Linter disabled:              false
  Organize imports disabled:    false
  VCS disabled:                 true

Linter:
  JavaScript enabled:           true
  JSON enabled:                 true
  CSS enabled:                  true
  GraphQL enabled:              false
  Recommended:                  false
  All:                          false
  Enabled rules:
  performance/noDelete
  suspicious/noCatchAssign
  suspicious/noUnsafeNegation
  style/useImportType
  complexity/noMultipleSpacesInRegularExpressionLiterals
  a11y/useValidLang
  complexity/noUselessEmptyExport
  suspicious/useNamespaceKeyword
  suspicious/useValidTypeof
  a11y/useValidAriaRole
  correctness/noConstantCondition
  a11y/useAriaActivedescendantWithTabindex
  suspicious/noAssignInExpressions
  style/useDefaultParameterLast
  complexity/noEmptyTypeParameters
  correctness/noConstructorReturn
  style/useSelfClosingElements
  suspicious/noDuplicateParameters
  suspicious/noDuplicateSelectorsKeyframeBlock
  correctness/noUnknownProperty
  style/useTemplate
  correctness/noUnusedLabels
  correctness/noUnreachableSuper
  suspicious/noCompareNegZero
  suspicious/noExplicitAny
  correctness/noSwitchDeclarations
  a11y/noAutofocus
  correctness/noUnsafeOptionalChaining
  correctness/noConstAssign
  suspicious/noControlCharactersInRegex
  complexity/noUselessTypeConstraint
  style/noVar
  suspicious/noDoubleEquals
  suspicious/noRedundantUseStrict
  style/useLiteralEnumMembers
  suspicious/noGlobalIsNan
  suspicious/noEmptyInterface
  suspicious/noConstEnum
  suspicious/noMisleadingCharacterClass
  correctness/noPrecisionLoss
  a11y/noLabelWithoutControl
  suspicious/noRedeclare
  correctness/noStringCaseMismatch
  correctness/noSetterReturn
  correctness/noInvalidConstructorSuper
  suspicious/noImplicitAnyLet
  suspicious/noFallthroughSwitchClause
  suspicious/noUnsafeDeclarationMerging
  correctness/noUnreachable
  a11y/useKeyWithClickEvents
  suspicious/noDuplicateObjectKeys
  complexity/noUselessThisAlias
  complexity/noThisInStatic
  complexity/useOptionalChain
  correctness/noInnerDeclarations
  style/noParameterAssign
  suspicious/noDuplicateCase
  a11y/useValidAnchor
  complexity/useRegexLiterals
  correctness/noSelfAssign
  correctness/noInvalidBuiltinInstantiation
  style/noUselessElse
  style/useShorthandFunctionType
  suspicious/noShadowRestrictedNames
  correctness/noInvalidDirectionInLinearGradient
  suspicious/noImportantInKeyframe
  a11y/useMediaCaption
  complexity/noUselessLabel
  complexity/noUselessCatch
  correctness/noUnsafeFinally
  a11y/useAriaPropsForRole
  correctness/noNonoctalDecimalEscape
  style/useEnumInitializers
  a11y/useHtmlLang
  suspicious/noDuplicateTestHooks
  complexity/noStaticOnlyClass
  style/useWhile
  suspicious/noConsole
  complexity/useArrowFunction
  style/noInferrableTypes
  a11y/noNoninteractiveTabindex
  complexity/useSimpleNumberKeys
  correctness/useYield
  a11y/noInteractiveElementToNoninteractiveRole
  style/useNumericLiterals
  correctness/noUnnecessaryContinue
  suspicious/noApproximativeNumericConstant
  suspicious/noImportAssign
  suspicious/noLabelVar
  correctness/noGlobalObjectCalls
  suspicious/useDefaultSwitchClauseLast
  a11y/useAltText
  correctness/noEmptyCharacterClassInRegex
  correctness/noUnknownUnit
  suspicious/noSparseArray
  a11y/useIframeTitle
  complexity/noBannedTypes
  a11y/noSvgWithoutTitle
  correctness/noVoidElementsWithChildren
  style/useAsConstAssertion
  correctness/useJsxKeyInIterable
  style/useExportType
  complexity/noUselessLoneBlockStatements
  suspicious/noDebugger
  style/noArguments
  a11y/useValidAriaValues
  suspicious/noCommentText
  a11y/useFocusableInteractive
  correctness/noUnmatchableAnbSelector
  suspicious/noMisleadingInstantiator
  suspicious/noDuplicateJsxProps
  suspicious/noGlobalAssign
  a11y/noPositiveTabindex
  correctness/noEmptyPattern
  complexity/noExcessiveNestedTestSuites
  security/noDangerouslySetInnerHtmlWithChildren
  a11y/useKeyWithMouseEvents
  suspicious/noExtraNonNullAssertion
  suspicious/noPrototypeBuiltins
  correctness/noRenderReturnValue
  correctness/useExhaustiveDependencies
  security/noGlobalEval
  style/noNonNullAssertion
  a11y/noRedundantRoles
  complexity/useFlatMap
  correctness/useIsNan
  style/useConst
  suspicious/noGlobalIsFinite
  suspicious/noSelfCompare
  suspicious/noShorthandPropertyOverrides
  suspicious/noAsyncPromiseExecutor
  suspicious/noDuplicateFontNames
  suspicious/noThenProperty
  suspicious/useGetterReturn
  security/noDangerouslySetInnerHtml
  style/useNodejsImportProtocol
  a11y/noDistractingElements
  suspicious/noArrayIndexKey
  complexity/noWith
  suspicious/noDuplicateClassMembers
  complexity/noExtraBooleanCast
  performance/noAccumulatingSpread
  a11y/useValidAriaProps
  a11y/noRedundantAlt
  correctness/noChildrenProp
  correctness/noUnknownFunction
  correctness/noInvalidPositionAtImportRule
  suspicious/noConfusingLabels
  suspicious/noSuspiciousSemicolonInJsx
  suspicious/noConfusingVoidType
  suspicious/noFocusedTests
  a11y/useButtonType
  a11y/useSemanticElements
  a11y/noAriaUnsupportedElements
  correctness/noInvalidGridAreas
  style/useFilenamingConvention
  correctness/noFlatMapIdentity
  a11y/noBlankTarget
  a11y/useHeadingContent
  correctness/useValidForDirection
  correctness/noVoidTypeReturn
  correctness/noInvalidUseBeforeDeclaration
  a11y/noAriaHiddenOnFocusable
  a11y/useGenericFontNames
  correctness/noUnknownMediaFeatureName
  a11y/useAnchorContent
  complexity/noUselessRename
  style/useNumberNamespace
  complexity/noUselessConstructor
  a11y/noAccessKey
  style/useExponentiationOperator
  style/noUnusedTemplateLiteral
  complexity/noUselessSwitchCase
  style/useSingleVarDeclarator
  suspicious/noExportsInTest
  a11y/noNoninteractiveElementToInteractiveRole
  style/noCommaOperator
  suspicious/noDuplicateAtImportRules
  suspicious/useIsArray
  a11y/noHeaderScope
  complexity/noUselessFragments
  suspicious/noMisrefactoredShorthandAssign
  complexity/noForEach
  suspicious/noClassAssign
  suspicious/noEmptyBlock
  suspicious/noFunctionAssign

Workspace:
  Open Documents:               0

Rule name

useFilenamingConvention

Playground link

Not reproducible in playground

Expected result

There is a convenient convention to name migration files using a number prefix (usually amount of unix seconds).
Something like this:
image

But somehow useFilenamingConvention is throwing errors on these files:

E.g. if creating file 123_abc.ts, getting error:

app/base/123_abc.ts lint/style/useFilenamingConvention ━━━━━━━━━━

  βœ– The filename should be in kebab-case or snake_case or PascalCase.
  
  β„Ή The filename could be renamed to one of the following names:
    123-abc.ts
    123_abc.ts
    123Abc.ts

Considering this is a bug cause biome itself suggesting 123_abc.ts as a valid file name.

Also there is an issue with bracket-wrapped files,
[id].ts works well, however [page_id].ts throws error:

app/base/[page_id].ts lint/style/useFilenamingConvention ━━━━━━━━━━

  βœ– The filename should be in kebab-case or snake_case or PascalCase.
  
  β„Ή The filename could be renamed to one of the following names:
    _page_id.ts
    -page-id.ts
    PageId.ts

Same error for [_id].ts and [page-id].ts

Somehow related to #3353 and #3650

Please advise.
Thanks.

Here is my config:

{
  "$schema": "https://biomejs.dev/schemas/1.9.0/schema.json",
  "formatter": {
    "indentStyle": "space",
    "indentWidth": 2
  },
  "linter": {
    "enabled": true,
    "rules": {
      "complexity": { "noUselessTernary": "off", "useLiteralKeys": "off" },
      "style": {
        "useFilenamingConvention": {
          "level": "error",
          "options": {
            "strictCase": true,
            "requireAscii": true,
            "filenameCases": ["kebab-case", "snake_case", "PascalCase"]
          }
        }
      },
      "suspicious": {
        "noConsole": "error"
      }
    }
  }
}

Code of Conduct

  • I agree to follow Biome's Code of Conduct
@sleewoo
Copy link
Author

sleewoo commented Sep 17, 2024

tried to reproduce on stackblitz but getting a cli error when running biome lint:

~/projects/vitejs-vite-b2fajw
❯ biome lint
/home/projects/vitejs-vite-b2fajw/node_modules/@biomejs/cli-linux-x64/biome: line 1: a command can only contain words and redirects

https://stackblitz.com/edit/vitejs-vite-b2fajw?file=biome.json

@ematipico
Copy link
Member

That's OK, this one of those rules where the playground can't help.

To create a reproduction, you can use our new CLI tool, that should help you to create very quickly repository with what you need: npm create @biomejs/biome-reproduction

@sleewoo
Copy link
Author

sleewoo commented Sep 17, 2024

@sleewoo
Copy link
Author

sleewoo commented Sep 17, 2024

when running lint task getting errors on files i created:

Screenshot_20240917_130058

@Conaclos Conaclos self-assigned this Sep 17, 2024
@Conaclos
Copy link
Member

In fact Biome categories the filename case as unknown: a filename using only digits is in unicase, a filename that starts with some digits and continue with letters/underscores/dashes is categorized as unknown. The current behavior is thus "intended".

I am unsure if we should recognize 44_name, 44-name, 44Name as snake_case, kebab-case, camelCase/PascalCase.
This is particularly problematic for the last one: is the name in camelCase or PascalCase?
Any opinion?

@sleewoo
Copy link
Author

sleewoo commented Sep 17, 2024

makes sense.
perhaps the underscore prefix is the solution?
if file starts with an underscore, should it be treated as snake_case?
even if prefix followed by digits.
_44_name looks like a perfect snake_case candidate to me.

@Conaclos
Copy link
Member

Conaclos commented Sep 17, 2024

perhaps the underscore prefix is the solution?

We are quite strict at the moment: Biome doesn't recognize a name prefixed with underscores, suffixed with underscores, or with double underscores as snake_case. However, useFilenamingConvention allows filename prefixed with underscores. Thus, before checking its case, useFilenamingConvention trims leading underscores.
Hence, that _44_name is trimmed to 44_name, which is recognized with an unknown case.

@Conaclos
Copy link
Member

I think it is quite common to prefix filenames with timestamps or ISO dates, we should somehow find a way to accept this pattern.

@Conaclos
Copy link
Member

Conaclos commented Sep 17, 2024

Here is my proposal: we assume that leading digits are in lowercase.

2024-09-17-filename, 2024_09_17_filename, 20240917FileName and 20240917filename are recognized as kebab-case, snake_case, camelCase and lowercase respectively.

2024FILENAME and 2024_FILE_NAME are not recognized as UPPERCASE and CONSTANT_CASE. They are categorized as unknown.

Implemented in #3957

@sleewoo what do you think?

@sleewoo
Copy link
Author

sleewoo commented Sep 17, 2024

awesome!
thanks

@sleewoo
Copy link
Author

sleewoo commented Sep 17, 2024

sorry, while on context... what about second part?
[id].ts vs [page_id].ts

have to add overrides for lot of route files with dynamic params (SolidJS)

@Conaclos
Copy link
Member

Fixed.
FYI [id].ts was already accepted. [page_id].ts was rejected because of the underscore.

@sleewoo
Copy link
Author

sleewoo commented Sep 17, 2024

perfect,
thank you much for such a fast resolution!

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 a pull request may close this issue.

3 participants