Skip to content

Ignore specific errors #11051

Description

This keeps getting asked on stackoverflow (just this morning : http://stackoverflow.com/a/39627892/390330) so thought there should be an issue to track it.

Ideation

Lots of linters do this:

  • Ignore errors for regions (using inline comments /** */).
  • Ignore errors per file (configured using tsconfig or command line)
  • Ignore certain error codes (configured using tsconfig or command line)

There needs to be 🚲 🏠 here so have at it 🌹

More

Activity

  1. mhegazy commented on Sep 22, 2016

    @mhegazy
    Contributor

    man.. this is one of my all time favorite topics :) As I mentioned in #9448 (comment), #8855 (comment), #6771 (comment), #3691 (comment), and i am sure there is more, errors are not the problem, and suppressing them is not the solution; this analogues of switching off your fire alarm. The real problem is you have a faulty fire alarm. So instead of muffling the errors, I would like to know what is the specific error, and why you find it useless. then we can talk how we can make the experience better.

  2. yortus commented on Sep 22, 2016

    @yortus
    Contributor

    Instead of suppressing compiler errors, it would be interesting to discuss the pros/cons of being able to override compiler options at the file/function level, at least those options that have only local effect.

    The closest equivalent to this at the moment is to break up into multiple projects, each with their own tsconfig.json file. Perhaps that's the best solution for major splits (eg a project with both node (target: ES6) and browser (target: ES5) code).

    But sometimes I wish I could override an option in a few places without spliting up the project. E.g., I could set the compiler options in tsconfig.json to their strictest setting, and then opt-out of them in a few specific declarations that for whatever reason TypeScript can't analyse properly, or that I need to revisit, but I know are not errors.

    I think some people are not adopting the strictest flags at all, because they are all-or-nothing project-wide and bring up huge numbers of errors on their existing codebase.

  3. yortus commented on Sep 22, 2016

    @yortus
    Contributor

    A couple of examples to go with my previous comment:


    Here is a util function from bluebird:

    function toFastProperties(obj: any) {
        /*jshint -W027,-W055,-W031*/
        function FakeConstructor() {}
        FakeConstructor.prototype = obj;
        var l = 8;
        while (l--) new FakeConstructor();
        ASSERT("%HasFastProperties", true, obj);
        return obj;
        // Prevent the function from being optimized through dead code elimination
        // or further optimizations. This code is never reached but even using eval
        // in unreachable code causes v8 to not optimize functions.
        eval(obj);
    }

    It would be nice not to have to set allowUnreachableCode: true project-wide just because of this one special case. That would cause a loss of safety in the rest of the project. What if we could preface just this function with // @allowUnreachableCode: true, thereby preserving unreachability checking everywhere else in the project?


    Here is part of a function from the TypeScript compiler:

                switch (searchSpaceNode.kind) {
                    case SyntaxKind.MethodDeclaration:
                    case SyntaxKind.MethodSignature:
                        if (isObjectLiteralMethod(searchSpaceNode)) {
                            break;
                        }
                    // fall through
                    case SyntaxKind.PropertyDeclaration:
                    case SyntaxKind.PropertySignature:
                    case SyntaxKind.Constructor:
                    case SyntaxKind.GetAccessor:
                    case SyntaxKind.SetAccessor:
                        staticFlag &= getModifierFlags(searchSpaceNode);
                        searchSpaceNode = searchSpaceNode.parent; // re-assign to be the owning class
                        break;
                    case SyntaxKind.SourceFile:
                        if (isExternalModule(<SourceFile>searchSpaceNode)) {
                            return undefined;
                        }
                    // Fall through
                    case SyntaxKind.FunctionDeclaration:
                    case SyntaxKind.FunctionExpression:
                        break;
                    // Computed properties in classes are not handled here because references to this are illegal,
                    // so there is no point finding references to them.
                    default:
                        return undefined;
                }

    Setting noFallthroughCasesInSwitch: false project-wide would cause a loss of safety in the rest of the project. What if we could preface just this function with // @noFallthroughCasesInSwitch: false, thereby preserving fall-through checking everywhere else in the project?

  4. RyanCavanaugh commented on Sep 22, 2016

    @RyanCavanaugh
    Member

    I really appreciate the examples -- these are what we're looking for, to be sure! But I'm going to break character here and state my strong subjective opinion about user code, which I usually try hard not to do: Both of those examples are horrifying.

    Bluebird could just as easily use something like !!false && eval(obj) to avoid that error. Or wrap the whole thing in a meaningless try / catch, which deopts to my knowledge. Anything they do here is just super-suspect anyway as there are no guarantees V8 won't start doing smarter analysis and ignoring their unreachable eval. I would guess a hyper-low-level function like this (the 8-times loop to force a psuedoclass to be created... what?) occurs in extremely few libraries.

    Our own use of IFT here is awful -- the second fall-through doesn't even save any LOC compared to just writing break; at the // Fall-through comment! The first instance is somewhat more excusable but I would still prefer we just never do this.

    Again, my own opinion. I would like to see more examples and will try not to judge them so hard.

  5. yortus commented on Sep 22, 2016

    @yortus
    Contributor

    Ryan Cavanaugh (@RyanCavanaugh) how would you prefer to rewrite the first fall-through in the tsc example? OR just not use switch at all?

  6. RyanCavanaugh commented on Sep 22, 2016

    @RyanCavanaugh
    Member
                switch (searchSpaceNode.kind) {
                    case SyntaxKind.PropertyDeclaration:
                    case SyntaxKind.PropertySignature:
                    case SyntaxKind.Constructor:
                    case SyntaxKind.GetAccessor:
                    case SyntaxKind.SetAccessor:
                    case SyntaxKind.MethodDeclaration:
                    case SyntaxKind.MethodSignature:
                        if ((searchSpaceNode.kind === SyntaxKind.MethodDeclaration) || 
                            (searchSpaceNode.kind === SyntaxKind.MethodSignature)) {
                            if (isObjectLiteralMethod(searchSpaceNode)) {
                                break;
                            }
                        }                                            
                        staticFlag &= getModifierFlags(searchSpaceNode);
                        searchSpaceNode = searchSpaceNode.parent; // re-assign to be the owning class
                        break;
  7. kitsonk commented on Sep 22, 2016

    @kitsonk
    Contributor

    Also, personal opinion, --noFallthroughCasesInSwitch can easily be turned off and put in the domain of a linter, which is where I think it personally is better suited.

    As far unreachable code, I originally felt that it was the domain of a linter, but now seeing what CFA provides, I now feel that when you have a compiler the is evaluating the flow of your code, then unreachable code becomes an error, versus a stylistic issue, even if it is still valid at run-time. It is now clearly in the wheelhouse of TypeScript.

  8. yortus commented on Sep 22, 2016

    @yortus
    Contributor

    Ryan Cavanaugh (@RyanCavanaugh) Oh cool, I thought empty cases also triggered a fall-through error but they don't.

  9. yortus commented on Sep 22, 2016

    @yortus
    Contributor

    Bluebird could just as easily use something like !!false && eval(obj) to avoid that error.

    ...sure, until Anders adds some more CFA goodness that statically detects that as unreachable code.

  10. kitsonk commented on Sep 22, 2016

    @kitsonk
    Contributor

    ...sure, until Anders adds some more CFA goodness that statically detects that as unreachable code.

    Well, it could be argued there is a big difference between an unreachable statement and a short circuited expression logic. I think that might be the argument with !!false && eval(obj) in that I know many minifiers evaluate expression logic like that for short circuits and do dead code removal. On the other hand, that is true for unreachable code as well. In any case, it is a dog chasing its tail, put one compiler hint here to avoid another compiler making a mistake.

  11. yortus commented on Sep 22, 2016

    @yortus
    Contributor

    Kitson Kelly (@kitsonk) tsc has already started clamping down on side-effect free (SEF) expressions as per #10814. It could in future (I'm speculating) also complain about !!false && eval(obj) as a side-effect-free (i.e. useless) expression. Interestingly the PR that fixes #10814 uses allowUnreachableCode as the opt-out flag to suppress SEF errors.

  12. mhegazy commented on Sep 22, 2016

    @mhegazy
    Contributor

    One philosophical distinction between errors of the like of noImplicitAny, can not find module, or type not assignable to, and ones reported by --allowUnreachableCode, --noFallthroughCasesInSwitch, and --noImplicitReturns, is that the later are more of linter functionality, that we are adding since "we are already there".

    The compiler has done the control flow analysis, already so it might as well tell you you have an unreachable statement. these are not core to the type system, and are meant for the 90% case. if you want more customization for these you can switch them off entirely, and use a linter rule. linters already are built with exceptions for specific errors and different levels, etc..

    The other errors are really at the core of what the compiler does. for instance, if it does not know what shape a module has, it can not say anything about the imports, or how they are used, further use of these types would not be safe either, since an error type are just any.,the tools can not guarantee that renaming x in one place will be safe, etc.. at the core is the TypeScript value -proposition.

    Now, I am not saying all the errors that the compiler emit today are ideal. And this is why we should be having these discussions. and for instance, short-hand modules, and using of wild cards in module names were conceived because of users complaining about a too-stringent error message. if they were to switch that one off, they would have masked the real cause, and instead see a degradation of service in the compiler and the tools.

  13. nxpatterns commented on Sep 26, 2016

    @nxpatterns

    The reason, why we would need such of feature is very simple:

    We have old applications, which we have to migrate step by step into a new framework. (An example would be migrating old JavaScript/TypeScript/.. Codes from Angular1 to 2)

    In the earlier steps we have to suppress some errors like this[foo] = bar and some other ones, so that the app is first runnable. If we can run the app in the new framework, we get the decision from Marketing/Management Department for the refactoring in depth. Marketing department is absolutely not interested in compiler errors.

    We have a pre-configured build system (big company) and may only change tsconfig.json. Currently the build sytem drums out our app because of existing errors. And there are a big amount of source codes and we have not the resources to correct every error in a short given time. Therefore we need some feature like to deactive certain errors, which were ok at that time (at the time of writing of that old stuff and the list is long.

    As you see, there are certain use cases, that are not considered by the creators of TypeScript :) The world is not perfect and you may not always expect perfect pre-conditions. Therefore the ability to deactivate (be it very important) errors is an essential feature for the real world.

    Thank you!

  14. mhegazy commented on Sep 29, 2016

    @mhegazy
    Contributor

    As you see, there are certain use cases, that are not considered by the creators of TypeScript :) The world is not perfect and you may not always expect perfect pre-conditions. Therefore the ability to deactivate (be it very important) errors is an essential feature for the real world.

    The typescript code base does not have --noImplicitThis or --strictNullChecks enabled for the same reason. so is not too strange of a scenario to the core team. what we have done instead is try with the new flag on, try to fix issues as much as possible, then iterate. once all issues have been fixed flip the switch. that has worked in the past for different new switches, we would hope it works with strinctNull as well.

  15. nxpatterns commented on Oct 1, 2016

    @nxpatterns

    Dear Mohamed, thank you very much for your feedbacks but I can not emphasize that enough;

       I want to suppress, the error, which I want. That can be any of them.  
    

    I appreciate the work from Microsoft very much, TS has a huge potential to be THE LANGUAGE OF WEB, but there is still no complete list, a kind of itemization of Error-Codes (TSWhatever <--> TSLint-Rule <--> Available Custom Rules <--> ...), we have to fish out them out of the source code. I think, there is still much work to do.

  16. 11 remaining items

  17. studds commented on Feb 16, 2017

    @studds

    if you want more customization for these you can switch them off entirely, and use a linter rule. linters already are built with exceptions for specific errors and different levels, etc.. (#11051 (comment))

    While I agree with this in principle, in practice it's difficult as the TSLint team is gradually removing linting rules as they get added to tsc - for example palantir/tslint#661 deprecates no-unreachable and palantir/tslint#1481 deprecates no-unused-variable.

    I agree that ideally errors should be tuned and not ignored; that less than ideal code should be improved and not papered over; and that a dedicated linter should be used for maximum flexibility.

    However, if the compiler is going to stray into the traditional territory of a linter, and especially where this encourages linters to remove rules, wouldn't it be pragmatic to add some small part of the flexibility a linter provides?

  18. RyanCavanaugh commented on Feb 16, 2017

    @RyanCavanaugh
    Member

    For lint-type rules, we either a) do add a configuration option and/or workaround ("no unused" is project-configurable and parameters can be prefixed with _) or b) consider the code to be unjustifiably suspicious (e.g. if (expr); {).

  19. studds commented on Feb 17, 2017

    @studds

    I did not mean to overlook the existing configuration options when I suggested the compiler should "add some small part of the flexibility a linter provides". I suppose that I mean specific options should be available per file / function / line, as you are able to do with a linter.

  20. abettadapur commented on Feb 23, 2017

    @abettadapur

    Another example, where I need to disable a rule per file.

    React requires you to declare this object to use the Context feature, which is never used throughout the file. When NoUnusedLocals is turned on, this throws a compiler error because it is never directly used in the file.

    private static childContextTypes: React.ValidationMap<any> = 
    {
            functionA: React.PropTypes.func.isRequired,
            functionB: React.PropTypes.func.isRequired,
            functionC: React.PropTypes.func.isRequired
    };
    
  21. grovesNL commented on Mar 1, 2017

    @grovesNL

    There are many TypeScript code generation tools that have issues with this as well. In some cases the code generation has to do quite a lot of effort in order to determine which parameters/locals are unused. This may not even be possible depending on which extension points are exposed, because the code generator may not have enough information to make this decision.

    With TSLint this was never a problem because the code generator could include /*tslint:disable*/ at the top of generated files. Now that people are moving to the TypeScript rules the code generators have to try to enforce every rule that consumers might happen to use in their projects.

    Besides code generation, personally I was in the same position as Dominic Black (@DomBlack) and wanted to enable every strict rule because they're extremely useful. However I found myself toggling the rules on and off constantly while making commits so it wasn't worth the effort. It's just not worthwhile for existing (sizable) code bases to enable these rules unless the rules can be toggled per file.

  22. mhegazy commented on Apr 21, 2017

    @mhegazy
    Contributor

    Seems to be the same issue as #9448. Closing in favor of #9448.

  23. sebdoucet commented on Jan 22, 2018

    @sebdoucet

    An other example with code generation with polymer...

    @Polymer.decorators.observe("items")
    private _onItemListChanged(newList: ListItemSource, oldList: ListItemSource): void {

    Compilation fail: '_onItemListChanged' is declared but its value is never read.

    Very sad to have no choice but changing the scope of the function.

  24. dyst5422 commented on Feb 13, 2018

    @dyst5422

    How about in the case that the compiler does not work correctly?

    Specific example being #21699

    There needs to be a way to work around these issues temporarily. And saying that it still emits JS we can use is not sufficient as a lot of tooling depends on the compiler information and having red squigglies all over my screen for issues that are not actual problems (neither from a runtime or type-checking standpoint) is incredibly challenging.

    Having the ability to disregard a list of error codes doesn't affect the type-checking capabilities either.

  25. thw0rted commented on Apr 25, 2018

    @thw0rted

    Hi Mohamed Hegazy (@mhegazy), I don't think this is strictly the same issue as #9448. This issue asked for:

    If this is the best issue to "ignore errors per file" then please re-open it, or direct me to an issue specifically about that capability as I haven't found one (or I could certainly open a new one).

    I was hoping to find some kind of an exclude pattern for strict checks, along the lines of:

    "compilerOptions": {
      "strict": {
        "exclude": ["./lib/notMyProblem/**", "../someProjectWithGeneratedCode/**"]
      }
    }
    

    I was hoping to import from 3rd party or generated TS and not worry about quality checks, while doing strict checking on all the code that is "my responsibility". I found this issue because jsweet makes "pretty good" TS but it lights up the compiler with dozens of strict-mode failures, and as an earlier comment suggests it might not be possible to avoid them.

  26. mhegazy commented on Apr 25, 2018

    @mhegazy
    Contributor

    I was hoping to import from 3rd party or generated TS and not worry about quality checks, while doing strict checking on all the code that is "my responsibility".

    consider using --noLibCheck to avoid checking and reporting errors in .d.ts files.

  27. thw0rted commented on Apr 26, 2018

    @thw0rted

    In my particular case, I'm importing from the TS files directly, rather than having to keep a separate tsc --watch running as I make changes to both the consuming project and the (peer project) library. If I pre-compiled the library and used the resulting .d.ts files, --noLibCheck would help. Maybe my use case (all straight TS, running on ts-node, some of which is "mine" and some of which is "third party") isn't the way it's intended to be used?

  28. mhegazy commented on Apr 26, 2018

    @mhegazy
    Contributor

    if you are using .ts files, they really become "your sources". so if the files are auto-generated, and have errors, that seems to me to be a bug in the generator.

  29. thw0rted commented on Apr 27, 2018

    @thw0rted

    I'm going to have to think about the cleanest way forward here. It's kind of a perfect storm of edge cases.

    I'm porting somebody else's Java library to run in Node, and used jsweet to get a head start. The TS output is "pretty good" but has some linter / strict checking issues, and I think it would unreasonably complicate the conversion logic to make the output pass tsc strict checks. I would compile those generated TS files to JS in a more permissive mode and then use them, but I want to write my consuming code in TS also so I was running everything in ts-node. That's a first for me so I'm not sure how well it would handle imports from TS-to-JS compiled code vs importing directly from the TS, in terms of debugging / sourcemaps.

    The way I see it, I can:

    • Keep everything purely TS, and disable strict checking (but miss obvious errors in my own code)
    • Keep everything TS, with strict checking, and hand-jam fixes into the TS generated by jsweet (time consuming)
    • Compile the library TS to JS and refer to that, using --noLibCheck (and maybe have sourcemap complications with ts-node)
    • Show up here and request a feature to selectively enforce strictness (very little downside for yours truly 😁 )
  30. locked and limited conversation to collaborators on Jul 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions