Skip to content

Commit

Permalink
Pkg.activate inside macros should still deactivate nbpkg, fixes #1475
Browse files Browse the repository at this point in the history
Had to force push, you better do something silly @fonsp
  • Loading branch information
dralletje authored Sep 24, 2021
1 parent a8ab180 commit b918c1c
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 1 deletion.
40 changes: 39 additions & 1 deletion src/analysis/ExpressionExplorer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,35 @@ function strip_indexing(x, inside::Bool=false)
end
end

"Module so I don't pollute the whole ExpressionExplorer scope"
module MacroHasSpecialHeuristicInside
import ...Pluto
import ..ExpressionExplorer, ..SymbolsState

"""
Uses `cell_precedence_heuristic` to determine if we need to include the contents of this macro in the symstate.
This helps with things like a Pkg.activate() that's in a macro, so Pluto still understands to disable nbpkg.
"""
function macro_has_special_heuristic_inside(; symstate::SymbolsState, expr::Expr)::Bool
# Also, because I'm lazy and don't want to copy any code, imma use cell_precedence_heuristic here.
# Sad part is, that this will also include other symbols used in this macro... but come'on
local fake_cell = Pluto.Cell()
local fake_reactive_node = Pluto.ReactiveNode(symstate)
local fake_expranalysiscache = Pluto.ExprAnalysisCache(
parsedcode=expr,
module_usings_imports=ExpressionExplorer.compute_usings_imports(expr),
)
local fake_topology = Pluto.NotebookTopology(
nodes=Pluto.DefaultDict(Pluto.ReactiveNode, Dict(fake_cell => fake_reactive_node)),
codes=Pluto.DefaultDict(Pluto.ExprAnalysisCache, Dict(fake_cell => fake_expranalysiscache))
)

return Pluto.cell_precedence_heuristic(fake_topology, fake_cell) < 8
end
# Having written this... I know I said I was lazy... I was wrong
end


###
# MAIN RECURSIVE FUNCTION
###
Expand Down Expand Up @@ -381,7 +410,16 @@ function explore!(ex::Expr, scopestate::ScopeState)::SymbolsState
# "You should make a new function for that" they said, knowing I would take the lazy route.
for arg in ex.args[begin+1:end]
macro_symstate = explore!(arg, ScopeState())
union!(symstate, SymbolsState(macrocalls=macro_symstate.macrocalls))

# Also, when this macro has something special inside like `Pkg.activate()`,
# we're going to treat it as normal code (so these heuristics trigger later)
# (Might want to also not let this to @eval macro, as an extra escape hatch if you
# really don't want pluto to see your Pkg.activate() call)
if arg isa Expr && MacroHasSpecialHeuristicInside.macro_has_special_heuristic_inside(symstate=macro_symstate, expr=arg)
union!(symstate, macro_symstate)
else
union!(symstate, SymbolsState(macrocalls=macro_symstate.macrocalls))
end
end

# Some macros can be expanded on the server process
Expand Down
24 changes: 24 additions & 0 deletions test/ExpressionExplorer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,30 @@ Some of these @test_broken lines are commented out to prevent printing to the te
macrocalls=[Symbol("@parent"), Symbol("@child"), Symbol("@grandchild")],
)
end
@testset "Macros and heuristics" begin
@test test_expression_explorer(
expr=:(@macro import Pkg),
macrocalls=[Symbol("@macro")],
definitions=[:Pkg],
)
@test test_expression_explorer(
expr=:(@macro Pkg.activate("..")),
macrocalls=[Symbol("@macro")],
references=[:Pkg],
funccalls=[[:Pkg, :activate]],
)
@test test_expression_explorer(
expr=:(@macro Pkg.add("Pluto.jl")),
macrocalls=[Symbol("@macro")],
references=[:Pkg],
funccalls=[[:Pkg, :add]],
)
@test test_expression_explorer(
expr=:(@macro include("Firebasey.jl")),
macrocalls=[Symbol("@macro")],
funccalls=[[:include]],
)
end
@testset "String interpolation & expressions" begin
@test testee(:("a $b"), [:b], [], [], [])
@test testee(:("a $(b = c)"), [:c], [:b], [], [])
Expand Down

1 comment on commit b918c1c

@dralletje
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AAAARG @fonsp

Please sign in to comment.