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

Atualizar regras de lint e corrigir problemas apontados pelo DeepScan #1629

Merged
merged 2 commits into from
Feb 21, 2024

Conversation

Rafatcb
Copy link
Collaborator

@Rafatcb Rafatcb commented Feb 18, 2024

Mudanças realizadas

O principal motivo para eu ter adicionado o eslint-plugin-jest ao projeto é que já vi fazerem um commit contendo um teste com .only, e eu também quase fiz isso sem querer. Aproveitei para adicionar outras regras que fazem sentido, como erros em console.log, alertas em condicionais em testes (que foi algo debatido no PR #1601), o no-dupe-keys mencionado no issue #1338 etc.

Adicionei duas novas configurações em extends (eslint:recommended e plugin:jest/recommended), e isso é um trade-off: por um lado, perdemos controle sobre mudanças de regras habilitadas/desabilitadas ao mudar a versão; por outro, ganhamos praticidade em não precisar manter uma lista longa de regras no .eslintrc e estamos usando "padrões recomendados".

Um exemplo de erro que essa modificação de configuração já pegou foi o throw error que estava fora do catch na função findUserByUsernameOrEmail, no arquivo /models/recovery.js.

Precisei modificar algumas expressões regulares por causa do escape desnecessário (no-useless-escape), e tem essa resposta do Stack Overflow como material de apoio para entender as alterações. Se, mesmo assim, não se sentirem confortáveis com essas mudanças, posso desabilitar a regra no arquivo remove-markdown.js, que é o arquivo que mais foi impactado, ou desabilitar de forma global (não acho recomendável).

Aproveitei para atualizar dependências relacionadas ao processo de lint.

Resolve #1338 — ao mexer nas regras do ESLint eu acabei resolvendo algumas coisas desse issue, então resolvi cuidar dele por completo.

Tipo de mudança

  • Correção de bug
  • Novas regras de lint

Checklist:

  • As modificações não geram novos logs de erro ou aviso (warning).
  • Tanto os novos testes quanto os antigos estão passando localmente.

Use ESLint recommended rules and also add new ones.
@Rafatcb Rafatcb added bug Comportamento diferente do esperado testes Criação ou melhoria de testes labels Feb 18, 2024
Copy link

vercel bot commented Feb 18, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
tabnews ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 19, 2024 11:25pm

Copy link
Collaborator

@aprendendofelipe aprendendofelipe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Show @Rafatcb! 💪💪💪

Fix some issues highlighted by DeepScan. Some of them do not have a real impact, for example an
unnecessary validation, but others do, such as access to a variable that can be undefined.
@Rafatcb
Copy link
Collaborator Author

Rafatcb commented Feb 19, 2024

Aproveitei para corrigir um link inválido no Markdown, que foi identificado pelo Codacy.

Copy link
Collaborator

@aprendendofelipe aprendendofelipe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bora pro merge?

@Rafatcb
Copy link
Collaborator Author

Rafatcb commented Feb 21, 2024

@aprendendofelipe vamos 😄

@aprendendofelipe aprendendofelipe merged commit 47ab2cf into main Feb 21, 2024
7 checks passed
@aprendendofelipe aprendendofelipe deleted the update-lint-rules branch February 21, 2024 23:36
@aprendendofelipe
Copy link
Collaborator

Em produção! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Comportamento diferente do esperado testes Criação ou melhoria de testes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[OTIMIZACAO] DeepScan report
2 participants