TS 1.8.x: invalid this reference inside arrow function

[Wintermoose]

TypeScript Version:
1.8.0 (through npm), 1.8.2 (visual studio)

Code

namespace Bug {

    export interface IDef {
        test2: () => any;
        test: (e:any) => any;
    }

    class TestController {
        protected from: number;

        public prepareDefinition(def: IDef) {
            return def;
        }

        public prepared = this.prepareDefinition({
            test2: () => {
                var q = this.from; // uses _this;
                var helper = () => {
                    var q2 = this.from; // uses _this
                };
                return {
                    fn1: (v) => {
                        var q3 = this.from; //uses this
                    },
                    fn2: helper
                }
            },
            test: (e) => { }
        } 
        );


    }

}

The code is compiled to ES5
Expected behavior:
All the 3 assignments (q, q2, q3) should compile to using _this

Actual behavior:
The q3 assignment refers to "this". The code above (with the exception of the helper(), which I kept for comparison) seems to be the minimal code to trigger the issue - even removing test() or its parameters (or parameter of fn1) results in what I believe is the correct code (using _this). The code also compiles correctly in 1.7.x

[sandersn]

This happens because when we check context sensitive functions inside an object literal we short-circuit checking of the return type because it might be circular. But the short-circuit means that the checker doesn't go back to check the function later (because it's in a call to checkExpressionCached), which means that this doesn't get marked as needing the _this = this emit.

Thanks @vladima for the psychic debugging here. We came up with two fixes with two variants:

1. Call checkNodeDeferred in the short-circuit logic in checkFunctionExpressionOrObjectLiteralMethod, same as it is for the non-short-circuit path.
2. When aggregating return types, call checkExpression instead of checkExpressionCached.

1a. Only call checkNodeDeferred in checkFunctionExpressionOrObjectLiteralMethod if the node hasn't already been added to the deferred node list. Maybe using a nodeflags or something.
2a. Only call checkExpression when the contextual mapper is identityMapper.

@ahejlsberg, thoughts on which is the best fix? I'll send out a PR with (1) since it's the simplest.

[sandersn]

PR #7232 is out for this now.

[Merenwen]

This appears to still be broken with compile on save in VS. Below is a screenshot of two files that were generated almost seconds apart with compile on save. The only real difference was the insertion of a line break.

The most frustrating part of this is that it appears to be non deterministic. Sometimes saving certain files will cause this issue, other times repeating the save generates the JavaScript correctly.

(Apologies for the ancient diff tool; red is expected good output, yellow is the incorrect output)/

General
Typescript VS Build: 1.8.31.0
IDE: Visual Studio 2015 R2
Targeting: ECMAScript 5

Module System
Module System: None

Output
Keep comments in JavaScript output: true
Combine JavaScript output into file: true
Generate declaration files: true
Do not emit outputs if any errors are reported: true