facebookarchive/prepack

Review all FatalError throws

Open

#2.020 aberto em 24 de mai. de 2018

Ver no GitHub
 (2 comments) (2 reactions) (0 assignees)JavaScript (14.268 stars) (520 forks)batch import
bughelp wanted

Description

I've seen a few code snippets like this:

    // todo: emit a diagnostic messsage
    throw new FatalError();

This is problematic for two reasons:

  1. User doesn't get any useful feedback.
  2. Worse, on the command-line it will show up as an internal error, because there must not be a FatalError without a CompilerDiagnostic thingy having been issued.

Concrete work items:

  1. Review all places in the code where a FatalError is thrown, and make sure that a corresponding CompilerDiagnostic thingy was issued before.
  2. Rethink best practices, as it is too easy to create mismatches between FatalError and CompilerDiagnostic (how did this get past code review?).

Guia do colaborador

Pilha de tecnologia
javascript
Domain
developer experiencetooling
Tipo Issue
refactor
DifficultyDificuldade de implementação estimada para um novo contribuidor, de 1 para alterações muito pequenas a 5 para trabalho de nível especializado.
3
Tempo estimadoUm intervalo de tempo aproximado para um colaborador experiente investigar, implementar, testar e preparar um pull request.
half day
Status da atividadeQuão disponível o issue aparece agora: novo, ativo, obsoleto, bloqueado ou aguardando entrada do mantenedor.
stale
ClarityCom que clareza o issue explica a mudança esperada, os critérios de aceitação e a próxima etapa.
mostly clear
Prerequisites
Understanding of FatalError and CompilerDiagnostic in prepack
Simpatia para novatosUma pontuação de 1 a 100 que estima o quão acessível este issue é para colaboradores iniciantes.
30
Direção de pesquisa
Review all occurrences of `throw new FatalError()` in the prepack codebase. For each, check if a `CompilerDiagnostic` is emitted before the throw. The issue mentions that some places have a TODO comment without a diagnostic. Look at the linked PRs or comments for any prior discussion. Files likely include compiler error handling modules.