Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

🐛 Enabling one nursery rule causes all other nursery rules to be enabled #4479

Closed
1 task done
Vivalldi opened this issue May 15, 2023 · 8 comments · Fixed by #4490 or #4511
Closed
1 task done

🐛 Enabling one nursery rule causes all other nursery rules to be enabled #4479

Vivalldi opened this issue May 15, 2023 · 8 comments · Fixed by #4490 or #4511
Assignees
Labels
A-Project Area: project configuration and loading S-Bug: confirmed Status: report has been confirmed as a valid bug

Comments

@Vivalldi
Copy link
Contributor

Environment information

CLI:
  Version:                      12.1.0
  Color support:                true

Platform:
  CPU Architecture:             aarch64
  OS:                           macos

Environment:
  ROME_LOG_DIR:                 unset
  NO_COLOR:                     unset
  TERM:                         "xterm-256color"
  JS_RUNTIME_VERSION:           "v18.14.0"
  JS_RUNTIME_NAME:              "node"
  NODE_PACKAGE_MANAGER:         "yarn/3.3.1"

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

Workspace:
  Open Documents:               0

Discovering running Rome servers...

Running Rome Server: ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

i The client isn't connected to any server but rage discovered this running Rome server.

Server:
  Version:                      12.1.0
  Name:                         rome_lsp
  CPU Architecture:             aarch64
  OS:                           macos

Workspace:
  Open Documents:               0

Other Active Server Workspaces:

Workspace:
  Open Documents:               8
  Client Name:                  Visual Studio Code
  Client Version:               1.78.2

Rome Server Log:

! Please review the content of the log file before sharing it publicly as it may contain sensitive information:
  * Path names that may reveal your name, a project name, or the name of your employer.
  * Source code

What happened?

  1. Enabled lint/nursery/noAccumulatingSpread
  2. Ran rome check .
  3. Noticed that lint/nursery/useExhaustiveDependencies & useLiteralKeys began to be checked as well.

Diagnostics
I tried setting recommended to false but the same issue was present

Expected result

Only enabled nursery rules should be checked

Code of Conduct

  • I agree to follow Rome's Code of Conduct
@Vivalldi Vivalldi added the S-To triage Status: user report of a possible bug that needs to be triaged label May 15, 2023
@ematipico
Copy link
Contributor

ematipico commented May 15, 2023

@Vivalldi could you please share your configuration?

@Vivalldi
Copy link
Contributor Author

Vivalldi commented May 15, 2023

@ematipico, this config and file is the basic replication

{
  "$schema": "https://docs.rome.tools/schemas/12.1.0/schema.json",
  "organizeImports": {
    "enabled": false
  },
  "linter": {
    "enabled": true,
    "rules": {
      "recommended": true,
      "nursery": {
        "noAccumulatingSpread": "error"
      }
    }
  }
}

test file showing 3 non-enabled nursery rules

const bannedType: Boolean = true;

if (true) {
	const obj = {};
	obj["useLiteralKey"];
}

Here's a repo as well - https://github.com/vivalldi/rome-4479

@Conaclos
Copy link
Contributor

Conaclos commented May 15, 2023

As a temporary workaround, you can add "recommended": false under linter.nursery:

{
  "$schema": "https://docs.rome.tools/schemas/12.1.0/schema.json",
  "organizeImports": {
    "enabled": false
  },
  "linter": {
    "enabled": true,
    "rules": {
      "recommended": true,
      "nursery": {
+       "recommended": false,
        "noAccumulatingSpread": "error"
      }
    }
  }
}

@Conaclos Conaclos added S-Bug: confirmed Status: report has been confirmed as a valid bug A-Linter Area: linter and removed S-To triage Status: user report of a possible bug that needs to be triaged A-Linter Area: linter labels May 15, 2023
@ematipico ematipico added the A-Project Area: project configuration and loading label May 16, 2023
@ematipico
Copy link
Contributor

I have an idea of the issue! It should be easy to fix

@Vivalldi
Copy link
Contributor Author

Vivalldi commented May 18, 2023

Looks like this is back in 12.1.2. Nursery rules should probably never respect the root level recommended.

Updated replication repo to 12.1.2 and was getting the same issue. Workaround of recommended: false in nursery still works

@ematipico
Copy link
Contributor

With the code you provided, what's the expected behaviour? Zero diagnostics?

@ematipico ematipico reopened this May 18, 2023
@Vivalldi
Copy link
Contributor Author

Vivalldi commented May 18, 2023

Correct, zero diagnostics. Below are two truth tables based on how I expect recommendations to behave. The only case that differs between them is the 4th case. Feel free to let me know if this doesn't line up with the project's expectations

Normal groups

All groups respect linter.rules.recommended unless there is an explicit group level setting.

linter.rules.recommended linter.rules.[group].recommended group recommended rules enabled
false unset No
false false No
false true Yes
true unset Yes
true false No
true true. Yes

Nursery group

The nursery group never respects linter.rules.recommended. It should only ever use it's group level setting.

linter.rules.recommended linter.rules.nursery.recommended group recommended rules enabled
false unset No
false false No
false true Yes
true unset No
true false No
true true. Yes

@ematipico
Copy link
Contributor

Perfect, I believe I know what's the source of the issue

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Project Area: project configuration and loading S-Bug: confirmed Status: report has been confirmed as a valid bug
Projects
None yet
3 participants