Code-Inspect / flowr

A program slicer and dataflow analyzer for the R programming language.
https://github.com/Code-Inspect/flowr/wiki
GNU General Public License v3.0
14 stars 2 forks source link

Dataflow for assign in condition produces incorrect node #813

Closed Ellpeck closed 1 month ago

Ellpeck commented 1 month ago

The test

        assertDataflow(label('assign in condition', ['name-normal', 'lambda-syntax', 'numbers', 'if', 'newlines', 'assignment-functions', 'strings', 'normal-definition', 'implicit-return', 'call-normal']),
            shell, `a <- \\() 2
if(y) {
   assign("a", function() 1)
}
a()`,  emptyGraph()
                .use('5', 'y')
                .call('4', '<-', [argumentInCall('0'), argumentInCall('3')], { returns: ['0'], reads: [BuiltIn] })
                .argument('4', ['3', '0'])
                .call('15', 'assign', [argumentInCall('9'), argumentInCall('13')], { returns: ['9'], reads: [BuiltIn], environment: defaultEnv().defineFunction('a', '0', '4'), onlyBuiltIn: true, controlDependencies: ['17'] })
                .argument('15', ['13', '9'])
                .call('16', '{', [argumentInCall('8')], { returns: ['8'], reads: [BuiltIn], controlDependencies: ['17'], environment: defaultEnv().defineFunction('a', '0', '4') })
                .argument('16', '8')
                .argument('17', '5')
                .argument('17', '16')
                .call('17', 'if', [argumentInCall('5'), argumentInCall('16'), EmptyArgument], { returns: ['16'], reads: ['5', BuiltIn], onlyBuiltIn: true, environment: defaultEnv().defineFunction('a', '0', '4', ['17']) })
                .call('19', 'a', [], { returns: ['1', '11'], reads: ['9', '0'], environment: defaultEnv().defineFunction('a', '9', '15', ['17']).defineFunction('a', '0', '4', ['17']) })
                .calls('19', ['3', '13'])
                .constant('1', undefined, false)
                .defineFunction('3', ['1'], {
                    out:               [],
                    in:                [{ nodeId: '1', name: undefined, controlDependencies: [] }],
                    unknownReferences: [],
                    entryPoint:        '1',
                    graph:             new Set(['1']),
                    environment:       defaultEnv().pushEnv()
                })
                .defineVariable('0', 'a', { definedBy: ['3', '4'] })
                .constant('11', undefined, false)
                .defineFunction('13', ['11'], {
                    out:               [],
                    in:                [{ nodeId: '11', name: undefined, controlDependencies: [] }],
                    unknownReferences: [],
                    entryPoint:        '11',
                    graph:             new Set(['11']),
                    environment:       defaultEnv().pushEnv()
                })
                .defineVariable('9', '"a"', { definedBy: ['13', '15'], controlDependencies: ['17'] })
        )

runs correctly, but the graph it produces contains a node called r-got8 (and l-expected8 for the constructed graph), but the corresponding AST still contains a correct node for id 8 (RSymbol (8) assign).

Mermaid graphs (with a deliberate error introduced for quick access, y renamed to yy in expected):

AST

Dataflow comparison (see bottom right for each graph)

This test is also currently part of #800, so the branch can optionally be used for testing.

EagleoutIce commented 1 month ago

It should be noted that this works only with the controlDependencies update of #800, which currently is not on main.