Rename NullUtil.filter to NullUtil.filterList (#1327)

Summary:
The old name is misleading. There _is_ a function called `filter` on
options, but its type is `(Option<T>, (T -> boolean)) -> Option<T>`:

  - Java: <https://docs.oracle.com/javase/8/docs/api/java/util/Optional.html#filter-java.util.function.Predicate->
  - Rust: <https://doc.rust-lang.org/std/option/enum.Option.html#method.filter>
  - Haskell: <https://hackage.haskell.org/package/base-4.12.0.0/docs/Control-Monad.html#v:mfilter>
  - OCaml (Core): <https://ocaml.janestreet.com/ocaml-core/latest/doc/base/Base/Option/index.html#val-filter>

This is even inconsistent with SourceCred’s own documentation:
<126332096f/src/util/null.js (L31)>

In general, a function called `foo` on options where `foo` also exists
on lists has the meaning, “interpret an `Option<T>` as a subsingleton
list, apply `foo` to the list, and reinterpret as an option”. To choose
the same name a conflicting function is confusing.

The function that was wanted is really just a special case of `flatMap`.
For instance, in Java:

```
-> import java.util.stream.Collectors;

-> (List.of(Optional.of(3), Optional.empty(), Optional.of(2))
>>     .stream()
>>     .flatMap(Optional::stream)
>>     .collect(Collectors.toList()))
|  Expression value is: [3, 2]
|    assigned to temporary variable $2 of type List<? extends Object>
```

Yet some languages do provide it as a utility function: [`catMaybes`] in
Haskell, or [`List.filter_opt`] in OCaml (Core). For parallelism with
the latter, we define `NullUtil.filterList`.

[`List.filter_opt`]: https://ocaml.janestreet.com/ocaml-core/latest/doc/base/Base/List/#val-filter_opt
[`catMaybes`]: https://hackage.haskell.org/package/base-4.12.0.0/docs/Data-Maybe.html#v:catMaybes

Test Plan:
That `yarn flow` passes suffices.

wchargin-branch: filter-list
This commit is contained in:
William Chargin 2019-08-26 10:16:44 -07:00 committed by GitHub
parent 12a3321ea7
commit 909045a7ec
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 7 additions and 7 deletions

View File

@ -85,7 +85,7 @@ export async function load(
// than blocking on the plugins sequentially.
// Since plugins often perform rate-limited IO, this may be a big performance
// improvement.
const pluginGraphPromises: Promise<Graph>[] = NullUtil.filter([
const pluginGraphPromises: Promise<Graph>[] = NullUtil.filterList([
discourseGraph(),
githubGraph(),
]);

View File

@ -88,6 +88,6 @@ export function orElse<T>(x: ?T, defaultValue: T): T {
* the callback that was passed to filter. This method basically wraps filter
* in a type-aware way.
*/
export function filter<T>(xs: $ReadOnlyArray<?T>): T[] {
export function filterList<T>(xs: $ReadOnlyArray<?T>): T[] {
return (xs.filter((x) => x != null): any);
}

View File

@ -133,25 +133,25 @@ describe("util/null", () => {
});
});
describe("filter", () => {
describe("filterList", () => {
it("filters out undefined and null but not other falsey values", () => {
const x = [0, undefined, NaN, null, false, ""];
const f = NullUtil.filter(x);
const f = NullUtil.filterList(x);
expect(f).toEqual([0, NaN, false, ""]);
});
it("typechecks as expected", () => {
const rs: $ReadOnlyArray<?string> = ["foo", undefined];
const _: string[] = NullUtil.filter(rs);
const _: string[] = NullUtil.filterList(rs);
});
it("returns a copy of the original array", () => {
const as = [1, 2, 3];
const bs = NullUtil.filter(as);
const bs = NullUtil.filterList(as);
expect(as).not.toBe(bs);
});
it("doesn't allow bad coercions", () => {
const as = [1, "foo", 2];
// $ExpectFlowError
const _: number[] = NullUtil.filter(as);
const _: number[] = NullUtil.filterList(as);
});
});
});