JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.6k stars 5.48k forks source link

Tab completion on `a[1].` should use propertynames if defined (or return nothing) #38006

Open knuesel opened 4 years ago

knuesel commented 4 years ago

Tab completion can give different results for the same value, depending on the syntax: element.<tab-tab> vs array[1].<tab-tab>. Example:

julia> using CSV

julia> write("test.csv", "a,b\n1,2\n3,4\n");

julia> data = CSV.File("test.csv")
2-element CSV.File{false}:
 CSV.Row: (a = 1, b = 2)
 CSV.Row: (a = 3, b = 4)

julia> row = data[1];

julia> row.<TAB TAB>
a b

julia> data[1].<TAB TAB>
columns lookup   names    row

It's understandable: for row, Julia is given the instance so it can readily call propertynames. For data[1] Julia only has the element type. But the different behavior is confusing. I don't see a good reason to return the field names for completion on data[1], since these completions are invalid.

Could Julia call getindex when required to get the correct completions for data[1]? Or is there too much concern with side effects?

Otherwise, could Julia check if propertynames is defined? It could then disable completion on data[1], which would be less confusing I think?

mcabbott commented 4 years ago

I can't seem to run your example, is it showing fieldnames(typeof(data))? Edit -- or perhaps fieldnames(eltype(data))?

In other cases it seems to work as you would expect:

julia> v = view([(a=1, b=2)], :,1);

julia> v1 = v[1];

julia> v.
indices offset1  parent   stride1
julia> v[1].
a b
julia> v1.
a b
julia> fieldnames(eltype(v))
(:a, :b)

But, edit, this hasn't overloaded propertynames I guess.

knuesel commented 4 years ago

@mcabbott how does it fail? I just checked and it works on a clean Julia 1.5.2 installation (note that the two <TAB TAB> in the example should not be copy-pasted but replaced by user action).

Note that propertynames works on an instance so you should compare fieldnames(eltype(v)) with propertynames(v[1]). Both return the same in this case so the tab completion works as expected.

knuesel commented 4 years ago

@mcabbott here's a self-contained example:

julia> import Base.propertynames

julia> struct A
           x::Int
           y::Int
       end

julia> propertynames(::A, private=false) = (:a, :b);

julia> array = [A(0, 0)];

julia> value = array[1];

julia> value.
a b
julia> array[1].
x y

Here the completions shown for array[1]. are invalid.

mcabbott commented 4 years ago

Thanks! Sorry about the grumpy reception.

So here it looks like it's going by fieldnames(eltype(array)). Which would be invalid with, say,

Base.getproperty(a::A, s::Symbol) = s===:a ? getfield(a,:x) : s===:b ? getfield(a,:b) : error("no go")

Note that by contrast, getfielddoes seem to extract the element & its properties:

julia> t = (p=value, q=2)
(p = A(0, 0), q = 2)

julia> t.p.
a b

whereas getproperty doesn't get run -- here AA is like A without the ::Ints:

julia> d = AA(value, 3)
AA(A(0, 0), 3)

julia> d.
a b
julia> d.x.
a b
julia> d.a. # does nothing

julia> d.x
ERROR: no go

I can't really think of a situation where running getindex (or getproperty) when you press tab would be so expensive that you'd want to avoid it.

knuesel commented 4 years ago

@mcabbott no worries. Interesting that it works with named tuples On the other hand the issue is not limited to collections:

read(`ls`) |> x->AA(x,0).
x y

Or simply

A(0,0).
x y

Of course Julia can't get the property names since the instance is not constructed yet, but it shouldn't show x and y as completions.

mcabbott commented 4 years ago

Not trying to run these constructors makes sense. But it's not even obvious how to check whether there is a reason not to show the fields. If you only defined Base.propertynames(::A{Char,Int}), then should it conclude that this could possibly apply?

knuesel commented 4 years ago

No I'd say: if the instance is not available, I think Julia should make a best effort to determine if there is a non-default propertynames definition for the type (and if there is, show no completion). In your example there is no such definition for the type A{Int,Int} before . so it should use fieldnames as usual. What do you think?

mcabbott commented 4 years ago

The REPL does more inference than I realised, so maybe it can look up methods of propertynames, and disable tab completion if it can't be decided (and perhaps if only getproperty is overloaded):

julia> f(x::Int) = begin println("int"); (a=1, b=x, c=x^2) end;

julia> f(x::Float64) = begin println("float");  (x=1.0, y=x, z=x^2) end;

julia> f(c::Char) = rand()>0.5 ? (p=1, q=2) : (r=3, s=4);

julia> f(1).
a b  c
julia> f(2.0).
x y  z
julia> f('c').
knuesel commented 3 years ago

I stumbled on what is probably another facet of the same issue (i.e. caused by the REPL not executing getindex):

julia> using Makie

julia> scene = scatter([1,2], [3,4])

julia> scene[1].

The REPL gives no completion. And yet:

julia> fieldnames(typeof(scene[1]))
(:parent, :transformation, :attributes, :input_args, :converted, :plots)

(same result for propertynames which is not specialized, so it defers to fieldnames).

In this case the REPL should show some completions but doesn't.

The getindex method for Scene is defined as

getindex(scene::Scene, idx::Integer) = scene.plots[idx]

so presumably Julia could even determine the type by looking at the return type of getindex without calling it (calling fieldnames on this type would then give the correct completions).