zed-industries / zed

Code at the speed of thought – Zed is a high-performance, multiplayer code editor from the creators of Atom and Tree-sitter.
https://zed.dev
Other
46.57k stars 2.64k forks source link

Code actions on format is *really* broken #15057

Open GustavoMelloGit opened 1 month ago

GustavoMelloGit commented 1 month ago

Check for existing issues

Describe the bug / provide steps to reproduce it

Current zed settings:

{
  "theme": "Ayu Mirage",
  "ui_font_size": 13,
  "buffer_font_size": 13,
  "format_on_save": "on",
  "formatter": {
    "code_actions": {
      "source.organizeImports": true,
      "source.removeUnusedImports": true
    }
  },
  "languages": {
    "TypeScript": {
      "code_actions_on_format": {
        "source.fixAll.eslint": true
      }
    }
  }
}

Project Eslintrc.json

{
  "env": {
    "browser": true,
    "es6": true,
    "jest": true
  },
  "extends": [
    "plugin:react/recommended",
    "airbnb",
    "prettier",
    "plugin:react-hooks/recommended",
    "plugin:prettier/recommended"
  ],
  "globals": {
    "Atomics": "readonly",
    "SharedArrayBuffer": "readonly",
    "JSX": "readonly",
    "require": true,
    "vi": "readonly"
  },
  "parser": "@typescript-eslint/parser",
  "parserOptions": {
    "ecmaFeatures": {
      "jsx": true
    },
    "ecmaVersion": 2018,
    "sourceType": "module"
  },
  "plugins": ["react", "@typescript-eslint", "prettier"],
  "settings": {
    "import/resolver": {
      "node": {
        "extensions": [".js", ".jsx", ".ts", ".tsx"]
      },
      "typescript": {}
    }
  },
  "rules": {
    "no-use-before-define": "off",
    "@typescript-eslint/no-explicit-any": "warn",
    "import/no-extraneous-dependencies": [
      "error",
      {
        "devDependencies": true
      }
    ],
    "@typescript-eslint/no-use-before-define": ["error"],
    "import/extensions": [
      "error",
      "ignorePackages",
      {
        "js": "never",
        "jsx": "never",
        "ts": "never",
        "tsx": "never"
      }
    ],
    "max-len": [
      "warn",
      {
        "code": 150
      }
    ],
    "linebreak-style": 0,
    "no-unused-vars": 0,
    "@typescript-eslint/no-unused-vars": [
      "warn",
      {
        "argsIgnorePattern": "^_",
        "varsIgnorePattern": "^_",
        "caughtErrorsIgnorePattern": "^_"
      }
    ],
    "no-param-reassign": 0,
    "no-case-declarations": 0,
    "no-underscore-dangle": 0,
    "no-plusplus": 0,
    "jsx-a11y/click-events-have-key-events": 0,
    "jsx-a11y/no-noninteractive-element-interactions": 0,
    "jsx-a11y/label-has-associated-control": 0,
    "react/react-in-jsx-scope": 0,
    "react/jsx-filename-extension": [
      1,
      {
        "extensions": [".jsx", ".tsx"]
      }
    ],
    "import/prefer-default-export": 0,
    "react/jsx-props-no-spreading": 0,
    "react/require-default-props": 0,
    "react/jsx-one-expression-per-line": 0,
    "react/sort-comp": [
      2,
      {
        "order": ["instance-variables", "lifecycle", "static-methods", "everything-else", "render"]
      }
    ],
    "react/static-property-placement": 0,
    "react/prop-types": 0,
    "no-shadow": "off",
    "@typescript-eslint/no-shadow": ["error"],
    "no-nested-ternary": 0,
    "jsx-a11y/media-has-caption": 0,
    "import/no-unresolved": ["error", { "ignore": ["^virtual:"] }],
    "jsx-a11y/control-has-associated-label": 0,
    "react/function-component-definition": [
      2,
      {
        "namedComponents": "arrow-function",
        "unnamedComponents": "arrow-function"
      }
    ],
    "arrow-body-style": 0
  }
}

Project .editorconfig

root = true

[*]
indent_style = space
indent_size = 2
end_of_line = lf
charset = utf-8
trim_trailing_whitespace = false
insert_final_newline = false

Project .prettierrc

{
  "trailingComma": "es5",
  "tabWidth": 2,
  "semi": true,
  "singleQuote": true,
  "jsxSingleQuote": false,
  "endOfLine": "lf",
  "bracketSpacing": true,
  "bracketSameLine": false,
  "arrowParens": "avoid",
  "singleAttributePerLine": false,
  "printWidth": 100
}

Environment

Zed: v0.144.4 (Zed) OS: macOS 14.5.0 Memory: 24 GiB Architecture: aarch64

If applicable, add mockups / screenshots to help explain present your vision of the feature

https://github.com/user-attachments/assets/1b36567e-b2fa-4539-b221-3fc07ca1cc1e

If applicable, attach your Zed.log file to this issue.

Zed.log

[Zed.log](https://github.com/user-attachments/files/16355514/Zed.log)
eulke commented 1 month ago

This might be related to

It should be fixed in the tomorrow preview release

GustavoMelloGit commented 1 month ago

This might be related to this It should be fixed in the tomorrow preview release

It might actually solve in the next release. I commented out the following lines and it solved the issue:

"code_actions": {
  "source.organizeImports": true,
  "source.removeUnusedImports": true
}
notpeter commented 1 month ago

@GustavoMelloGit Can you confirm whether you still have this issue when using Zed Preview. You can safely install both Zed Stable and Zed Preview at the same time to test it out.

GustavoMelloGit commented 1 month ago

@notpeter Of course! Tested here and the issue seems to have changed. Now it does not add a ton of eslint-disable lines, but it also does not fix the issues (Same configs and project then before). If you want to, i can record it again.

Zed: v0.146.1 (Zed Preview)
OS: macOS 14.5.0
Memory: 24 GiB
Architecture: aarch64
mrnugget commented 1 month ago

So what exactly is the issue? That it adds the class again?

Also, is there a specific reason for why you have code_actions_on_format and code_actions as formatter? Why not merge them into one?

{
  "formatter": {
    "code_actions": {
      "source.organizeImports": true,
      "source.removeUnusedImports": true,
      "source.fixAll.eslint": true
    }
  }
}

If you do this, do you get the same behavior? If so, which code action do you have to comment out to make it work?

GustavoMelloGit commented 1 month ago

https://github.com/user-attachments/assets/738799a8-c4bd-4bcf-95c6-e85c328c122d

@mrnugget Using the settings that you sent i get this. It kinda works, but it is unstable. I was pressing cmd+s the entire time in the beginning and it was'nt fixing the issues. Also it does not fix some issues properly, as you can see on converting ' to ".

Zed: v0.146.1 (Zed Preview)
OS: macOS 14.5.0
Memory: 24 GiB
Architecture: aarch64
mrnugget commented 1 month ago

I wrote above:

If so, which code action do you have to comment out to make it work?

Did you try that? Play with different configurations to filter out which one's causing the issue?

(Maybe it's the combination of all three causing the issue, but that would be good to know too)

You wrote:

It kinda works, but it is unstable. I was pressing cmd+s the entire time in the beginning and it was'nt fixing the issues.

It only runs code_actions_on_format on cmd+s if the file is dirty and there's something to be saved. Sounds like you didn't have a dirty file and thus nothing was saved on cmd+s and thus no code actions ran.

Also it does not fix some issues properly, as you can see on converting ' to ".

I'm not sure I can spot exactly what the issue is. Can you show a clear case where that happens? You did change some " to ' at ~30s, but then a stray ' shows up and I don't know where that comes from.

GustavoMelloGit commented 1 month ago

Sounds like you didn't have a dirty file

I did have some prettier issues going on, as you can see: image

but then a stray ' shows up and I don't know where that comes from.

It added itself on cmd+s, it looks like this happens sometimes. Even if I comment out organizeImports and removeUnusedImports, it still occurs. I did some tests without these two settings, and it looks better but is not perfect. Do you want me to try a specific scenario?

mrnugget commented 1 month ago

Sounds like you didn't have a dirty file

I did have some prettier issues going on, as you can see:

That's not a dirty file though, no? A dirty file is one that has changes in it that haven't been saved yet. Like completion.rs here:

screenshot-2024-07-31-10 16 30@2x

Even if I comment out organizeImports and removeUnusedImports, it still occurs. I did some tests without these two settings, and it looks better but is not perfect. Do you want me to try a specific scenario?

So that means it's only the "source.fixAll.eslint": true action that causes the problems then?

GustavoMelloGit commented 1 month ago

So that means it's only the "source.fixAll.eslint": true action that causes the problems then?

In the stable version, source.organizeImports and source.removeUnusedImports make things worse. However, in Zed Preview, it seems that only source.fixAll.eslint causes issues on its own.

mrnugget commented 1 month ago

However, in Zed Preview, it seems that only source.fixAll.eslint causes issues on its own.

Can you provide a very small reproducible test case? i.e. a code file, an eslint config, a package json that shows the problem? Otherwise it's really hard to track this down.

GustavoMelloGit commented 1 month ago

@mrnugget at the beginning i have sent all my configs. Is there anything left? I guess providing a CodeSandBox link does not help much....

mrnugget commented 1 month ago

Yeah, you provided a 114-line ESLint configuration file 😄 That's not "small" to me, and I don't think it's a minimal reproducible example.