Skip to content

Commit

Permalink
hack warnings: disable individual warnings via .hhconfig flag
Browse files Browse the repository at this point in the history
Summary:
So far, `hack_warnings` flag could be set to `true`, `false` or a list of authorized codes. However, `hack_warnings` is currently set via JustKnobs, which does not allow to set it to a list of codes. So we set the type of `hack_warnings` back to being a book and add a new .hhconfig flag `disabled_warnings` to allow controlling which codes we want to generate.

I opted for a list of disabled warnings rather than enabled warnings because in the long run, I expect the list of disabled to be shorter than the list of enabled. Plus it makes it easier to add new warning codes.

Differential Revision: D62378235

fbshipit-source-id: 65ba672a229c270cd0a7ec6d7a2ec2d9310d5a9b
  • Loading branch information
Catherine Gasnier authored and facebook-github-bot committed Sep 12, 2024
1 parent d958996 commit 26a2168
Show file tree
Hide file tree
Showing 10 changed files with 76 additions and 18 deletions.
9 changes: 7 additions & 2 deletions hphp/hack/src/client_and_server/serverConfig.ml
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,6 @@ let load_config config options =
(bool_opt "populate_dead_unsafe_cast_heap" config)
?tco_log_exhaustivity_check:(bool_opt "log_exhaustivity_check" config)
?dump_tast_hashes:(bool_opt "dump_tast_hashes" config)
?hack_warnings:(all_or_some_ints_opt "hack_warnings" config)
?warnings_default_all:(bool_opt "warnings_default_all" config)
?tco_strict_switch:(bool_opt "strict_switch" config)
?tco_allowed_files_for_ignore_readonly:
Expand Down Expand Up @@ -605,7 +604,13 @@ let load
~tco_sticky_quarantine:local_config.lsp_sticky_quarantine
~tco_lsp_invalidation:local_config.lsp_invalidation
~tco_autocomplete_sort_text:local_config.autocomplete_sort_text
~hack_warnings:local_config.hack_warnings
~hack_warnings:
(if local_config.hack_warnings then
GlobalOptions.All_except
(int_list_opt "disabled_warnings" config
|> Option.value ~default:[])
else
GlobalOptions.NNone)
~warnings_default_all:local_config.warnings_default_all
~hh_distc_exponential_backoff_num_retries:
local_config.hh_distc_exponential_backoff_num_retries
Expand Down
3 changes: 1 addition & 2 deletions hphp/hack/src/client_and_server/serverLocalConfig.ml
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,7 @@ type t = {
(** POC: @ljw - controls how quarantine invalidates folded decls *)
autocomplete_sort_text: bool;
(** POC: @mckenzie - if true, autocomplete sorts using sort text attribute *)
hack_warnings: int GlobalOptions.all_or_some;
(** POC: @catg - turn on hack warnings. *)
hack_warnings: bool; (** POC: @catg - turn on hack warnings. *)
warnings_default_all: bool;
(** If true, `hh` is equivalent to `hh -Wall`, i.e. warnings are shown.
Otherwise, `hh` is equivalent to `hh -Wnone`, i.e. warnings are not shown. *)
Expand Down
4 changes: 2 additions & 2 deletions hphp/hack/src/client_and_server/serverLocalConfigLoad.ml
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ let default =
lsp_sticky_quarantine = false;
lsp_invalidation = false;
autocomplete_sort_text = false;
hack_warnings = GlobalOptions.All;
hack_warnings = true;
warnings_default_all = false;
}

Expand Down Expand Up @@ -806,7 +806,7 @@ let load_
config
in
let hack_warnings =
all_or_some_ints "hack_warnings" ~default:default.hack_warnings config
bool_ "hack_warnings" ~default:default.hack_warnings config
in
let zstd_decompress_by_file =
bool_
Expand Down
9 changes: 7 additions & 2 deletions hphp/hack/src/options/globalOptions.ml
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ type 'a all_or_some =
| ASome of 'a list
[@@deriving eq, show]

type 'a none_or_all_except =
| NNone
| All_except of 'a list
[@@deriving eq, show]

let default_saved_state =
{
loading = default_saved_state_loading;
Expand Down Expand Up @@ -176,7 +181,7 @@ type t = {
tco_lsp_invalidation: bool;
tco_autocomplete_sort_text: bool;
tco_extended_reasons: extended_reasons_config option;
hack_warnings: int all_or_some;
hack_warnings: int none_or_all_except;
warnings_default_all: bool;
tco_strict_switch: bool;
tco_allowed_files_for_ignore_readonly: string list;
Expand Down Expand Up @@ -285,7 +290,7 @@ let default =
tco_lsp_invalidation = false;
tco_autocomplete_sort_text = false;
tco_extended_reasons = None;
hack_warnings = All;
hack_warnings = All_except [];
warnings_default_all = false;
tco_strict_switch = false;
tco_allowed_files_for_ignore_readonly = [];
Expand Down
9 changes: 7 additions & 2 deletions hphp/hack/src/options/globalOptions.mli
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ type 'a all_or_some =
| ASome of 'a list
[@@deriving eq, show]

type 'a none_or_all_except =
| NNone
| All_except of 'a list
[@@deriving eq, show]

val default_saved_state : saved_state

val with_saved_state_manifold_api_key :
Expand Down Expand Up @@ -253,7 +258,7 @@ type t = {
tco_autocomplete_sort_text: bool;
tco_extended_reasons: extended_reasons_config option;
(** Controls whether we retain the full path for reasons or only simple witnesses *)
hack_warnings: int all_or_some; (** turn on hack warnings *)
hack_warnings: int none_or_all_except; (** turn on hack warnings *)
warnings_default_all: bool;
tco_strict_switch: bool;
(** Enable strict case checking in switch statements *)
Expand Down Expand Up @@ -371,7 +376,7 @@ val set :
?tco_lsp_invalidation:bool ->
?tco_autocomplete_sort_text:bool ->
?tco_extended_reasons:extended_reasons_config ->
?hack_warnings:int all_or_some ->
?hack_warnings:int none_or_all_except ->
?warnings_default_all:bool ->
?tco_strict_switch:bool ->
?tco_allowed_files_for_ignore_readonly:string list ->
Expand Down
27 changes: 25 additions & 2 deletions hphp/hack/src/oxidized/gen/global_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// This source code is licensed under the MIT license found in the
// LICENSE file in the "hack" directory of this source tree.
//
// @generated SignedSource<<c334712fa5641695c08f3750fe5ce336>>
// @generated SignedSource<<642754a417b3593b8bcb4cfa09c0f06c>>
//
// To regenerate this file, run:
// hphp/hack/src/oxidized_regen.sh
Expand Down Expand Up @@ -88,6 +88,29 @@ pub enum AllOrSome<A> {
ASome(Vec<A>),
}

#[derive(
Clone,
Debug,
Deserialize,
Eq,
EqModuloPos,
FromOcamlRep,
Hash,
NoPosHash,
Ord,
PartialEq,
PartialOrd,
Serialize,
ToOcamlRep
)]
#[rust_to_ocaml(attr = "deriving (eq, show)")]
#[repr(C, u8)]
pub enum NoneOrAllExcept<A> {
NNone,
#[rust_to_ocaml(name = "All_except")]
AllExcept(Vec<A>),
}

#[derive(
Clone,
Debug,
Expand Down Expand Up @@ -221,7 +244,7 @@ pub struct GlobalOptions {
pub tco_lsp_invalidation: bool,
pub tco_autocomplete_sort_text: bool,
pub tco_extended_reasons: Option<ExtendedReasonsConfig>,
pub hack_warnings: AllOrSome<isize>,
pub hack_warnings: NoneOrAllExcept<isize>,
pub warnings_default_all: bool,
pub tco_strict_switch: bool,
pub tco_allowed_files_for_ignore_readonly: Vec<String>,
Expand Down
4 changes: 2 additions & 2 deletions hphp/hack/src/oxidized/manual/global_options_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::gen::global_options::SavedStateLoading;
use crate::gen::package_info::PackageInfo;
use crate::gen::parser_options::ParserOptions;
use crate::gen::saved_state_rollouts::SavedStateRollouts;
use crate::global_options::AllOrSome;
use crate::global_options::NoneOrAllExcept;
use crate::i_set;
use crate::s_map;
use crate::s_set;
Expand Down Expand Up @@ -134,7 +134,7 @@ impl Default for GlobalOptions {
tco_lsp_invalidation: false,
tco_autocomplete_sort_text: false,
tco_extended_reasons: None,
hack_warnings: AllOrSome::ASome(vec![]),
hack_warnings: NoneOrAllExcept::AllExcept(vec![]),
warnings_default_all: false,
tco_strict_switch: false,
tco_allowed_files_for_ignore_readonly: vec![],
Expand Down
6 changes: 3 additions & 3 deletions hphp/hack/src/typing/typing_warning_utils.ml
Original file line number Diff line number Diff line change
Expand Up @@ -438,9 +438,9 @@ let module_of_migrated

let code_is_enabled tcopt code =
match TypecheckerOptions.hack_warnings tcopt with
| GlobalOptions.All -> true
| GlobalOptions.ASome codes ->
List.mem codes (Codes.to_enum code) ~equal:Int.equal
| GlobalOptions.NNone -> false
| GlobalOptions.All_except disabled_codes ->
not (List.mem disabled_codes (Codes.to_enum code) ~equal:Int.equal)

let add tcopt (type a x) ((pos, kind, warning) : (x, a) Typing_warning.t) : unit
=
Expand Down
12 changes: 12 additions & 0 deletions hphp/hack/src/utils/config_file/rust/config_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,18 @@ impl ConfigFile {
})
}

pub fn get_ints_or(
&self,
key: &str,
default: Vec<isize>,
) -> Result<Vec<isize>, std::num::ParseIntError> {
self.map.get(key).map_or(Ok(default), |s| {
s.split_terminator(',')
.map(|s| s.trim().parse())
.collect::<Result<_, _>>()
})
}

pub fn get_float(&self, key: &str) -> Option<Result<f64, std::num::ParseFloatError>> {
self.map.get(key).map(|s| s.parse())
}
Expand Down
11 changes: 10 additions & 1 deletion hphp/hack/src/utils/hh_config/hh_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use oxidized::decl_parser_options::DeclParserOptions;
use oxidized::experimental_features;
use oxidized::global_options::ExtendedReasonsConfig;
use oxidized::global_options::GlobalOptions;
use oxidized::global_options::NoneOrAllExcept;
use oxidized::parser_options::ParserOptions;
use package::PackageInfo;
use sha1::Digest;
Expand Down Expand Up @@ -432,7 +433,15 @@ impl HhConfig {
else { None }
}
}),
hack_warnings: hhconfig.get_all_or_some_ints_or("hack_warnings", default.hack_warnings)?,
hack_warnings: {
let is_on = hh_conf.get_bool_or("hack_warnings", true)?;
if is_on {
let disabled_warnings = hhconfig.get_ints_or("disabled_warnings", vec![])?;
NoneOrAllExcept::AllExcept(disabled_warnings)
} else {
NoneOrAllExcept::NNone
}
},
warnings_default_all: hhconfig.get_bool_or("warnings_default_all", default.warnings_default_all)?,
tco_strict_switch: hhconfig.get_bool_or("strict_switch", default.tco_strict_switch)?,
tco_package_v2: hhconfig.get_bool_or("package_v2", default.tco_package_v2)?,
Expand Down

0 comments on commit 26a2168

Please sign in to comment.