Fix compiler warnings for ant clean compile [LUCENE-7602]

[asfimport]

---

Migrated from LUCENE-7602 by Paul Elschot, 2 votes, resolved Apr 19 2017
Attachments: LUCENE-7602.patch (versions: 3), LUCENE-7602-ContextMap-lucene.patch, LUCENE-7602-ContextMap-solr.patch
Linked issues:

• Change raw Map type to Map<String,Object> for ValueSource context [LUCENE-5335] #6399
• fix java7 warnings in the source code [LUCENE-3538] #4612
• Replace ValueSource context Map with a more concrete data type [LUCENE-6232] #7294

[asfimport]

Paul Elschot (migrated from JIRA)

Patch of 24 Dec 2016.

This consists of:

• replacing Map by Map<?,?> in a few submodules.
• 7 @SuppressWarnings for unchecked casts (see also below),
• a split off of Const*DocValues classes into their own source files,
• one removal of close() on an AutoClosable,
• a few minor generics improvements,

I have a question on these cases, there are 7 of them:

  `@SuppressWarnings`("unchecked")
   public void createWeight(Map<?,?> context, IndexSearcher searcher) throws IOException {
   // FIXME: how to use a helper method here to avoid the unchecked cast? See https://docs.oracle.com/javase/tutorial/java/generics/capture.html
     ((Map<Object,Object>)context).put("searcher",searcher);

I tried to implement such a helper function, but I could not get it to compile cleanly.
Any suggestions for this?

[asfimport]

Paul Elschot (migrated from JIRA)

The patch also contains a few similar fixes for test code, but it is too much work to be complete for the test code, so I left it at that.

[asfimport]

Paul Elschot (migrated from JIRA)

This ContextMap lucene patch includes the previous patch of 24 December 2016.
This also changes all use of Map for function queries to ContextMap, which changes the public API, although not much.
All lucene tests pass.

[asfimport]

Paul Elschot (migrated from JIRA)

The ContextMap solr patch needs the ContextMap lucene patch.
It uses ContextMap in solr, also changing public API.
All solr tests pass.

[asfimport]

Paul Elschot (migrated from JIRA)

The interesting pieces of code for ContextMap:

The class itself:

public class ContextMap extends HashMap<Object,Object> {
  public ContextMap() { // empty
  }

  public ContextMap(ContextMap source) { // new copy
    super(source);
  }

  public ContextMap(IdentityHashMap<?,?> source) { // for solr
    super(source);
  }
}

the way it is used in solr, in SlotAcc.java:

class SolrContextMap extends ContextMap {
  SolrContextMap(org.apache.solr.search.QueryContext context) {
    super((java.util.IdentityHashMap)context); // CHECKME: copy ok?
  }
}

// TODO: we should really have a decoupled value provider...
// This would enhance reuse and also prevent multiple lookups of same value across diff stats
abstract class FuncSlotAcc extends SlotAcc {
  protected final ValueSource valueSource;
  protected FunctionValues values;

  public FuncSlotAcc(ValueSource values, FacetContext fcontext, int numSlots) {
    super(fcontext);
    this.valueSource = values;
  }

  `@Override`
  public void setNextReader(LeafReaderContext readerContext) throws IOException {
    values = valueSource.getValues(new SolrContextMap(fcontext.qcontext), readerContext);
  }
}

and some unchanged code from solr's QueryContext.java:

/*
 * Bridge between old style context and a real class.
 * This is currently slightly more heavy weight than necessary because of the need to inherit from IdentityHashMap rather than
 * instantiate it on demand (and the need to put "searcher" in the map)
 * `@lucene`.experimental
 */
public class QueryContext extends IdentityHashMap implements Closeable {
   ...
}

[asfimport]

Paul Elschot (migrated from JIRA)

The lucene patch replaces the use of IdentityHashMap in ValueSource.java by a ContextMap, because I think it is wrong to use an IdentityHashMap there. Strings are used as keys, and Strings are not unique objects in java.

However, this conflicts somewhat with the way ContextMap is used in solr. There QueryContext needs an IdentityHashMap, but the solr tests pass even with the patch applied.

[asfimport]

Paul Elschot (migrated from JIRA)

With both patches applied, ant clean compile-test gives a single warning.

Perhaps ContextMap should be separate issue.

For lucene I think it is worthwhile to use a lucene class without generics as the context map instead of the Map<?,?> variations that are present in the current code.

For solr I have no idea. I would hope that using IdentityHashMap is no more needed because queries are immutable.

[asfimport]

Uwe Schindler (@uschindler) (migrated from JIRA)

I am so happy that you are interested in fixing the function query warnings! 😆

[asfimport]

Uwe Schindler (@uschindler) (migrated from JIRA)

For solr I have no idea. I would hope that using IdentityHashMap is no more needed because queries are immutable.

Maybe Solr should also switch to ContextMap then?

[asfimport]

Ahmet Arslan (@iorixxx) (migrated from JIRA)

can't we just use Map<Object,Object> instead of Map<?,?> or ContextMap?

[asfimport]

Paul Elschot (migrated from JIRA)

Do you mean like this:

public class ContextMap extends AbstractMap<Object,Object>
{ ... }

[asfimport]

Paul Elschot (migrated from JIRA)

Meanwhile I tried implementing solr's QueryContext by extending ContextMap, and no more wrapping of fcontext.qcontext in a SolrContextMap, see FuncSlotAcc.setNextReader above.

The solr tests passed, so I think there is no more need for IdentityHashMap, in both lucene and solr.

Shall I post a complete patch against master, or just the changes changes since yesterday?

[asfimport]

Paul Elschot (migrated from JIRA)

Patch of 26 Dec 2016.
Mostly as discussed above.

ContextMap extends HashMap. I tried implementing AbstractMap, but that ends up in a detour to a HashMap anyway, so I left it at direct extension.

Is there a way to quickly check for unused imports at top level?
I used ant precommit for that, but it is quite slow because it stops after the first module with an error, and quite a few modules are involved here.

[asfimport]

Paul Elschot (migrated from JIRA)

can't we just use Map<Object,Object> ?

ContextMap implements that interface.
Since this is widely used, I prefer to use a lucene class (ContextMap) over an interface that is defined in the java language (Map<Object,Object>), because it allows a change in a single place.

We could still separate the implementation from the interface, but that would be more than fixing the compiler warnings here.

[asfimport]

Ahmet Arslan (@iorixxx) (migrated from JIRA)

I think the current issue will clean up three previous issues.