technomancy/slamhound

Confuses imports when multiple classes match

Closed this issue · 5 comments

We import (javax.xml.rpc.holders StringHolder) in one of our files. Slamhound rewrites this to an import of StringHolder from a CORBA package that is on the class path (but not imported anywhere).

Slamhound should prefer original imports if present (much like you had to do for :rename).

guns commented

This should have been addressed in the 1.5.0 release… could you please tell me the artifact that contains javax.xml.rpc.holders.StringHolder?

It's in axis-jaxrpc-1.4.jar.

Here's the ns declaration before:

(ns worldsingles.iov
  (:require [worldsingles.environment :refer [my-settings]])
  (import   java.net.URL
            javax.xml.rpc.holders.StringHolder
            (com.iesnare.www.dra.api.GetEvidenceDetails GetEvidenceDetailsResponseEvidence_detailsEvidence)
            (com.iesnare.www.dra.api.CheckTransactionDetails CheckTransactionDetailsResponseDetailsDetail
             holders.CheckTransactionDetailsResponseDetailsHolder)
            com.newrelic.api.agent.Trace))

and here's it after Slamhound:

(ns worldsingles.iov
  (:require [worldsingles.environment :refer [my-settings]])
  (:import (com.iesnare.www.dra.api.CheckTransactionDetails CheckTransactionDetailsResponseDetailsDetail)
           (com.iesnare.www.dra.api.CheckTransactionDetails.holders CheckTransactionDetailsResponseDetailsHolder)
           (com.iesnare.www.dra.api.GetEvidenceDetails GetEvidenceDetailsResponseEvidence_detailsEvidence)
           (java.net URL)
           (org.omg.CORBA StringHolder)))

Curiously, at this point if I fix the StringHolder type and run Slamhound again, it no longer gets confused. In other words, if I start from this:

(ns worldsingles.iov
  (:require [worldsingles.environment :refer [my-settings]])
  (:import (com.iesnare.www.dra.api.CheckTransactionDetails CheckTransactionDetailsResponseDetailsDetail)
           (com.iesnare.www.dra.api.CheckTransactionDetails.holders CheckTransactionDetailsResponseDetailsHolder)
           (com.iesnare.www.dra.api.GetEvidenceDetails GetEvidenceDetailsResponseEvidence_detailsEvidence)
           (com.newrelic.api.agent Trace)
           (java.net URL)
           (javax.xml.rpc.holders StringHolder)))

Then then the only thing Slamhound does is to remove the NewRelic Trace import (known issue in another ticket).

Perhaps the extra work Slamhound does to split out the holders package affects something in the algorithm?

guns commented

Thank you for the descriptive reply; I think I have a good idea of what's going on here. Be back in a few.

guns commented

Aha! Your ns :import statement reads:

(import …)

Instead of:

(:import …)

I suppose it wouldn't be hard for Slamhound to make up for the ns macro's extreme permissiveness…

Oh good catch! I hadn't noticed that difference! Thank you!