Add silence support.

[LegNeato]

With this patch there is a new section in .flowconfig:

[silence]
/path/to/file.js
/path/to/dir

Paths in [silence] will still be typechecked, but any
errors will be silenced as if there was a suppress_comment comment string
on every line where there are errors.

I called it silence to make it stand out from ignore and suppress_comment as
it currently silences all errors, including lints.

I believe this fixes #869, though I admit I didn't read
the whole issue.

(If this is accepted I'll add docs in a follow-up).

[LegNeato]

Note that I have never written OCaml before...let me know if there is anything I can do better.

[willmruzek]

I'm curious to see how this will be received by the flow team.

All for suppressing flow errors internal to dependencies, while also checking against the API.

[LegNeato]

Open questions:

• Should we accept regexes instead of paths?
  • I am leaning towards "no". Note that AFAIK users can still silence whole directories in the current state.
• Should we throw a warning when a file is silenced but there are no problems in the file to silence?
  • I am leaning towards "no", as silent means silent. But "yes" would mean that users would be alerted when their silences are no longer needed. "Yes" would also interact weird with directories and/or regexes.
• Should we to silence node_modules by default?
  • I am leaning towards "no", as it would be a big default behavior shift for flow. But "yes" would mean that less effort would be needed for developers that have a problem in a dep, google, find [silence] and add node_modules to it--a whole class of issues would no longer affect flow users.

[willmruzek]

Should we accept regexes instead of paths?

I would follow whatever [ignore] does.

Should we throw a warning when a file is silenced but there are no problems in the file to silence?

I agree, silent means silent.

Should we to silence node_modules by default?

Not at first. Let people get used to [silence]. Then down the road it can be a default.

[rcknight]

Silencing node_modules will fix the spam which has been putting me off from using flow - Looking forward to the merge, thanks @LegNeato

[LegNeato]

👉 @gabelevi

[zoontek]

No ETA on this? 😕

[alexmbarton]

@samwgoldman is this PR of interest to you?

[jlaustill]

I CAN'T use flow until this is merged, my company/boss simply won't allow it. This is THE feature that will make flow TAKE OFF. Anything I can do to help test/improve this feature please reach out to me. I want this!

[TrySound]

@jlaustill There's [untyped] section for this case. You can see reference in thread.

[jlaustill]

@TrySound I didn't notice that, thanks for pointing it out! I'll check it out. This has been frustrating me for quite awhile, and have not come across untyped.

EDIT:

quoting from that commit "Thanks to LegNeato for a similar PR in #4916. That PR proposes a [silence] section which still typechecks matching files but suppresses all errors. Whereas [untyped] treats the whole file as any, [silence] would treat only broken stuff as any, which is also useful."

This is not acceptable, treating everything as "any" defeats the entire purpose. I'll clone this pr branch and play with it. The silence behavior is what I need.

[jlaustill]

Has anyone looked at this in the last week?

[icaroharry]

+1 for this

[TrySound]

I don't think broken types should implicitly works and hide errors. It's like knowing there's a problem with your health but don't care about it. And one day you will die. Who cares?

[TrySound]

Don't shoot in your feet, guys.

[Palisand]

@TrySound That depends on your definition of "broken". If your project contains a dependency that is using older versions of flow (which is a common scenario), your project's flow version might consider that dependency's typings to be broken. However, they are still perfectly valid for the flow version used by the dependency and can be safely silenced.

[TrySound]

[untyped] safely treats it as any without hiding dangerous errors

[jlaustill]

@TrySound [untyped] would also ignore any types that WERE working, why would you EVER want to do that???

I just need to silence errors in node_modules that I have ZERO control over. If flow can get types and help me with the rest of the library I want that, hence [silence]. Give me the help, don't break my build because of errors which I can not control though...

[haggholm]

@TrySound What I (and I think many others) really want is for Flow to verify our use of 3rd party APIs, but not to make us responsible for 3rd party internals. [untyped] and [ignore] fails to do the former. Doing nothing does the latter. [silence] hits the sweet spot, insofar as it is possible.

[vincenzovitale]

+1000 for merging this.

[noahtallen]

Yeah, I'm seeing the same problem as @yamov. They all seem to be type errors coming from react-native-fbsdk and react-native, but the other third-party packages don't seem to be throwing any errors, which is good!

[LegNeato]

Still digging in, I don't really know OCaml so not sure where to make the fix. Rather than pollute this PR, I filed a new issue for those that want to keep track of the [declarations] fix for flow builtin types:

#6631

[nodkz]

I dislike [declarations] name for this option. It does not explain what it really means.

Small poll for option naming:
#6631 (comment)

[jedwards1211]

It doesn't suppress any parse errors on invalid files (that is what the existing [ignore] section is for).

Groaaaaaaaaan, have you not heard about all the time-wasting errors on purposely malformed test JSON files? So what we need now is yet another option....

[just STFU about]
<PROJECT_ROOT>/node_modules/.*\.json

[jedwards1211]

@nodkz that's to mimic the fact that probably no one would assume that [ignore] means "act like this file is missing"

I agree the name is a true WTF. It's like saying "these files are type declarations. Unlike all of my other type declaration files, which are intended for type checking, I'm explicitly declaring these declarations because obviously that means you shouldn't check these declarations"

[mrkev]

As stated above, it's meant to align with Hack's decl mode. From the docs:

Decl mode code is not typechecked. However, the signatures of functions, classes, etc in decl mode are extracted, and are used by the typechecker when checking other code. Decl mode is most useful when converting over code written in PHP: although the body of that code might have type errors that you don't want to deal with right away, it's still beneficial to make the signature of that code, or at the very least its mere existence, visible to other code.

That's also the idea behind surfacing parse errors: you can't extract signatures if you can't understand the code. It is purposefully different from [ignore].

[vladshcherbin]

@mrkev decl is pure shit title no one understands.

This poll gives you understanding what developers expect it to be called. Changing silence naming was a bad idea. Please stop acting like it's fine just because Hack language named it so.

[STRML]

I'll say it more gently, but I agree with @vladshcherbin. [declarations] sounds an awful lot like [libs] and does not meaningfully describe what it does. It also is confusing given Flow syntax like declare class or declare module.

[silence], while also not perfect due to the existence of [ignore] (which treats the module as completely missing), at least better describes what it does. I'd prefer something even more descriptive like [ignoreErrors].

[TrySound]

[flow_fix_me]: *

[jedwards1211]

@mrkev yeah I know, I already saw that, but how is Hack in any way relevant to this? (not to mention PHP, for goodness sake...)
It seems to me like you guys are so used to its terminology that you have a hard time seeing how confusing this will be to non-Hack users (i.e. most of us, probably).

[jedwards1211]

@mrkev it's also worth mentioning that all .js.flow files can be thought of as "declaration files" regardless of whether we want to suppress errors in them. "flow-typed declarations" is also a commonly-used term. And "type declarations" can even live in ordinary .js files.

[lll000111]

@mrkev Weighing whether yet another comment is justified, I think it is possibly okay to repeat what I wrote elsewhere, because I pointed out that the naming violates a principle that every programmer is familiar with: information hiding and not exposing internals.

In #6572 I wrote:

When this is documented some day I hope there will also be an explanation for the name "declarations". The previous choice "silence" was more or less obvious to anyone, "declarations" requires internal knowledge about "decl mode" that most Flow users are completely unaware of (I never heard about it before reading through the PR discussion). I think it's unfortunate that a naming that exposes internals was chosen,

I think one should aim to name things meant for external consumption based on the outside view, not based on what's (well hidden) inside.

[mrkev]

Unfortunately I didn't make the call on this one. Flow was born largely out of the Hack team (and in fact still a lot of code is shared) so to me it's not surprising they wanted to align things this way. IMO [silence] is a little removed from from the functionality this provides because it doesn't silence all errors: parse errors will still be as loud as always. Discussion about this is perhaps best had in a separate (open) issue though 👍

[jedwards1211]

@mrkev well I think most people here want it to silence parse errors as well. This BS with parse errors in intentionally malformed JSON has got to stop

[mrkev]

That's what [ignore] is for.

[TrySound]

And [untyped]. There is nothing to silence with parse errors. $FlowFixMe does work in this case too.

[lll000111]

@mrkev ignore ignores the whole thing. What I think people want is that internal issues are not reported. I cannot fix them. I still want to know about issues of how I use the API! I do care about he boundary layer. I do not care about what goes on behind it (I have no influence over it, not my package).

@TrySound Why did you downvote my comment? You accumulated an extraordinary amount of downvotes in this thread. Look at your own comments first, maybe if you have nothing positive to contribute stay out of this thread altogether if you don't like the topic that much?

$FlowFixMe does work in this case too.

Everybody knows that.

[jedwards1211]

@mrkev I don't think you realize how cumbersome it is to use [ignore] for that. What I wish I could do is

[ignore]
<PROJECT_ROOT>/node_modules/.*\.json

But that makes flow think all package.json files are missing, causing an error on every package I import from. I have to ignore on a package-by-package basis, which gets really old.

I just want a way to suppress all errors from node_modules/.*\.json files without causing any new errors, and I think it's bizarre that there's still no easy way to do it.

[jamesisaac]

Are people having success with using [declarations]? Attempting to use it on Flow 0.81 as described in the documentation, it's making no difference in error output for me.

@jedwards1211 I wouldn't have thought that would be a big issue:

1. Doesn't Flow only parse files under node_modules if they're referenced starting from your project? So a library would have to be importing its malformed json files for them to be picked up.

2. The ignore entries are OCaml regular expressions, so you could probably do something like <PROJECT_ROOT>/node_modules/[a-z0-9\-]+/(lib|src|test)/.*\.json to catch everything.

3. How many libraries do this anyway, that it would be too cumbersome to just add an ignore rule for each one? Don't think I've ever come across this type of error using Flow in many JS and Node projects.

[jedwards1211]

@jamesisaac it's not so much that tons of libraries have this problem, but rather that things like this which are stupidly cumbersome get more and more annoying over time. There's no 100% ideal solution I can copy into each of my projects.

<PROJECT_ROOT>/node_modules/[a-z0-9\-]+/(lib|src|test)/.*\.json wouldn't catch files in node_modules/foo/node_modules/bar/test/bad.json, so the character range should at least include /. Even this is not a foolproof solution, because should there be file with @flow that tries to require one of those ignored .json files will cause a Flow error. Probably not the case in any of my dependencies, but I am agitating for a 100% foolproof way to suppress the malformed JSON errors once and for all. There are almost never cases where I'd find it helpful to know something is wrong with a JSON file in a dependency.

AFAIK, Flow parses files under node_modules whether they're referenced or not.

[noahtallen]

The reality is that (it seems) a lot of us want type processing in node modules but NO errors shown in any node modules. We want errors to be shown in our own code when we missuse a module, but we don't want to see that any node modules did something wrong internally. We want to be able to maintain code that is error free and see that on a PR there are no flow errors, but we can't do that when node modules reports hundreds of errors for dependencies I can't control. Many of those errors for us are from Facebook modules, such as the react native fbsdk or react native itself. I still haven't found an easy way to accomplish this. Should be as easy as "If error originates from $glob, do not report or display"