Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

auto-completion of java method names for global symbols in repl buffer not working #2528

Closed
behrica opened this issue Nov 22, 2018 · 14 comments

Comments

@behrica
Copy link
Contributor

behrica commented Nov 22, 2018

Expected behavior

From the "compliment" documentation it should work to auto complete the methods of a var precisely
(meaning that it shows only applicable methods, which can be called on the object)

https://camo.githubusercontent.com/2739aa5c45d1f82542d9bc2c9e1f0433f94c9f44/68747470733a2f2f7261772e6769746875622e636f6d2f616c6578616e6465722d79616b75736865762f636f6d706c696d656e742f6d61737465722f646f632f696d616765732f6d656d6265722d636f6e746578742e706e67

Actual behavior

It shows a far longer list of possible methods, most completely irrelevant to the object

see screenshot
screenshot from 2018-11-22 23-30-07

Steps to reproduce the problem

The following code in the repl should be able to autocomplete all applicable methods for java.lang.String and nothing more

(def a-string "abc")
(.|  a-string)

(where | is the cursor position)

Instead it will list far more methods.

The tests of the "compliment" library suggest that this is supported:
https://github.com/alexander-yakushev/compliment/blob/558ccac9e46c39ad512ec200d175b9dc73547b5b/test/compliment/t_core.clj#L66

@behrica
Copy link
Contributor Author

behrica commented Nov 22, 2018

The completion middleware and completion library itself seem to work correctly, as
this code does work correctly

(with-open [conn (repl/connect :port 37719)]
     (-> (repl/client conn 1000)    ; message receive timeout required
       (repl/message {:op "complete" :symbol ".s" :context "(__prefix__ a-string)" :ns 'user })
       clojure.pprint/pprint))

@behrica
Copy link
Contributor Author

behrica commented Nov 22, 2018

Looking at this code (which extract the context), seem to suggest that it is not foreseen to work in the situation of above (completing a global var in the repl)
Tracing of the methods showed that it does not extract the required context "(__prefix__ a_string)" as it would be required for compliment to work correctly

(defun cider-completion-get-context-at-point ()
  "Extract the context at point.
If point is not inside the list, returns nil; otherwise return \"top-level\"
form, with symbol at point replaced by __prefix__."
  (when (save-excursion
          (condition-case _
              (progn
                (up-list)
                (check-parens)
                t)
            (scan-error nil)
            (user-error nil)))
    (save-excursion
      (let* ((pref-end (point))
             (pref-start (cider-completion-symbol-start-pos))
             (context (cider-defun-at-point))
             (_ (beginning-of-defun))
             (expr-start (point)))
        (concat (when pref-start (substring context 0 (- pref-start expr-start)))
                "__prefix__"
                (substring context (- pref-end expr-start)))))))

(defun cider-completion-get-context ()
  "Extract context depending on `cider-completion-use-context' and major mode."
  (let ((context (if (and cider-completion-use-context
                          ;; Important because `beginning-of-defun' and
                          ;; `ending-of-defun' work incorrectly in the REPL
                          ;; buffer, so context extraction fails there.
                          (derived-mode-p 'clojure-mode))
                     (or (cider-completion-get-context-at-point)
                         "nil")
                   "nil")))
    (message "context: %s " context)
    (if (string= cider-completion-last-context context)
        ":same"
      (setq cider-completion-last-context context)
      context)))

I work very much repl-first style, so basically all my code gets created in the repl and the copied to clojure files. So for me a very well working completion in the repl would be very useful, specially in java inter op scenarios.

It is clear that extracting the correct "context" for compliment to work, can be challenging.
But I think that the above use case (exact java method completion of global var) can be a low hanging fruit to get it working correctly

@alexander-yakushev
Copy link
Member

alexander-yakushev commented Nov 23, 2018

Ah, yes, that's true it is not supposed to work in the REPL buffer. I wrote that code originally, and my assumption was that code in the REPL buffer would most often not be a complete Clojure form for context to parse it (bacause people usually don't enable Paredit there).

Another way to tackle this would be to somehow solve alexander-yakushev/compliment#53.

@behrica
Copy link
Contributor Author

behrica commented Nov 23, 2018

I am just realizing that is does work in a clojure file....
I was assuming that the behavior would be the same in the repl vs a clojure buffer,
but this is not the case.

I need to confess, that I did not realize so far, that most of what I do in the repl, I could do in a clojure file.

And I have paredit in the repl enabled. Why would people not do so ?

My workflow so far was "repl buffer" focused. So I edited code in repl buffer and copy pasted it then into clojure files, when it worked.

I start to see only now, that I could work by starting with global vars in a clj file and move them then into methods, without hardly using the repl buffer
Is it possible that cider promotes this workflow , or focuses more on it ?

@alexander-yakushev
Copy link
Member

alexander-yakushev commented Nov 23, 2018

Is it possible that cider promotes this workflow , or focuses more on it ?

Well, nobody tries to enforce any particular workflow, and your approach is completely fine; yet indeed, writing and evaluating code in the buffer directly is usually much more convenient :).

My workflow so far was "repl buffer" focused. So I edited code in repl buffer and copy pasted it then into clojure files, when it worked.

Yeah, avoiding copy-pasting code is the primary reason why many developers develop and evaluate the code in Clojure code buffers. It's incredibly cool when you try it!

And I have paredit in the repl enabled. Why would people not do so ?

Last time I looked at that (and that was some long time ago), Paredit was confused by the REPL's current.namespace=> prompt. If it's no longer the case, perhaps there is no reason not to enable Paredit in the REPL buffer.

@behrica
Copy link
Contributor Author

behrica commented Nov 23, 2018

Ok, I made some more tests.
I tried the same "completion" in three different type of cider buffers.

The completion action always the same
(.| network)

"network" is a global var defined in the namespace I am "in".
of java class: org.deeplearning4j.nn.multilayer.MultiLayerNetwork

The buffer types which I tried where

  • repl
  • cider scratch pad
  • a repl file which imports the class in the name space declartation

so I excepted the same completions. But I got different results.

I got different results and had a hard time to understand, what they depend on.
It always worked, if I am in the "clj" file which has a name space declaration, which contains
(import org.deeplearning4j.nn.multilayer.MultiLayerNetwork)

Being in the cider scratch pad or the repl, did typically not work UNLESS
I changed the name space to the names space of the clj file.

Sometimes it did not work, but I had a hard time to reproduce it.
Finally I realized that is does not work, if I am in a new namespace which never imported in the past the java class, but uses the full qualified name:

(ns a-super-new-ns)
(def builder-2 (org.deeplearning4j.nn.conf.NeuralNetConfiguration$Builder.))
(..  builder-2)

The moment I type once the import:
(import 'org.deeplearning4j.nn.conf.NeuralNetConfiguration$Builder)

it started to work for that current name space.

Hhmm. I tried more stuff...

There is somewhere an edge case of mixing:

  • using different name spaces
  • creating java objects using full qualified class names
  • import statements of java classes
  • order of doing the things above
  • being in either
    • repl
    • clojure file of a name space with import statement
    • scratch pad (so without ns declaration)

where I end in a situations where a var is defined, but the completion does not work.
It seems to work clearly better in this order:

  • "clojure file of a ns with the java imports"
  • scratch pad (so without ns declaration even in same name space as the clojure file)
  • repl

In the first two it mostly works, I would say.

@behrica
Copy link
Contributor Author

behrica commented Nov 23, 2018

I found one reproducible szenario which does not work.

  1. start a fresh clojure repl with cider-jack-in , (no specific project required)
  2. create a scratchpad with cider-scratch

evaluate:

(def blob (javax.sql.rowset.serial.SerialBlob. (byte-array 1)))

#'user/blob
(. | blob)   // try to complete

Here the completion does not work.

Now, after having imported the class, it will work on the same var:

(import javax.sql.rowset.serial.SerialBlob)
javax.sql.rowset.serial.SerialBlob
(. | blob)

From "now on" I can create new vars either with the full qualified name or the class name only,
it works in any case.

So the completion depends somehow on the "state" of the ns,
(regarding the imports)
and not only on the "class" of the object.

@behrica
Copy link
Contributor Author

behrica commented Nov 23, 2018

I could verify the same behavior with

compliment.sources.class-members/members-candidates

Should we move the discusion there ?

It seems to be related to the import caching done in "populate-members-cache",
which tries to get all potential methods from all imports from the name space.

In my case, the "import" is not in the ns-map, as I use a full qualified class name to create the object.

@alexander-yakushev
Copy link
Member

Yes, the class member completion is not expected to work at all unless the class is imported. It can be made to work in case when context with a global Var is available; but I haven't gotten to that yet.

@alexander-yakushev
Copy link
Member

There was also a bug in caching imported classes which I fixed only yesterday, and it is not in CIDER yet.

@behrica behrica changed the title auto-completion of java method names for global symbols not working auto-completion of java method names for global symbols in repl buffer not working Nov 23, 2018
@behrica
Copy link
Contributor Author

behrica commented Nov 23, 2018

Yes, the class member completion is not expected to work at all unless the class is imported. It can be made to work in case when context with a global Var is available; but I haven't gotten to that yet.

Should this maybe be documented ? I would think that via reflection we can get the required information, independent of the class being imported or not. That it depends as well on the state of the name space is unexpected.

I changed as well the title of this issue.
In reality we have 2 separate issues, i think:

  1. global var completion in cider-repl does not work as good as in a clojure buffer.
    (independently of the import of the class or not)
    trivial example, which does work in clj buffers, but not in repl:
user> (def foo "foo")
#'user/foo
user> (.| foo) 
  1. global var completion depends on the "imported classes" into the name space and not only on the class of the object to complete methods for

@alexander-yakushev
Copy link
Member

I'd rather not document it but added this is a feature. I'll try that sometime soon.

@behrica
Copy link
Contributor Author

behrica commented Nov 23, 2018

I'd rather not document it but added this is a feature. I'll try that sometime soon.

Great !
I created an issue for this:
alexander-yakushev/compliment#58

@bbatsov bbatsov closed this as completed Jan 2, 2019
@stardiviner
Copy link
Contributor

Is it possible to make CIDER completion work for two dots .. operator?

(.. (java.util.Date.) |toString (|endsWith "2019"))

Here is an example where | is the place of point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants