Bug 332772 - Support problem filters for API Use Scan Task
Summary: Support problem filters for API Use Scan Task
Status: RESOLVED FIXED
Alias: None
Product: PDE
Classification: Eclipse Project
Component: API Tools (show other bugs)
Version: 3.7   Edit
Hardware: PC Windows XP
: P3 enhancement (vote)
Target Milestone: 4.3 M5   Edit
Assignee: Curtis Windatt CLA
QA Contact:
URL:
Whiteboard:
Keywords: noteworthy
Depends on:
Blocks: 221877 397604
  Show dependency tree
 
Reported: 2010-12-16 12:40 EST by Peter Parapounsky CLA
Modified: 2013-01-08 06:43 EST (History)
7 users (show)

See Also:


Attachments
Work In Progress (16.65 KB, patch)
2011-04-08 14:24 EDT, Curtis Windatt CLA
no flags Details | Diff
WIP (17.93 KB, patch)
2011-04-15 15:59 EDT, Curtis Windatt CLA
no flags Details | Diff
wip2 (53.00 KB, patch)
2012-09-21 14:25 EDT, Michael Rennie CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Parapounsky CLA 2010-12-16 12:40:21 EST
Build Identifier: 

The primary function of this scan is to report referenced API. However it also reports internal and illegal API references. But it doesn't allow specifying filter(s) as does the API Analysis task. Which is a problem in our headless builds as developers might set such filters in their IDE and get different results from what they will see in the automated builds API Use scans.

Reproducible: Always
Comment 1 Curtis Windatt CLA 2010-12-20 14:33:14 EST
We are considering this for M6, but it requires more investigation.

Currently the analysis task supports problem filters.  The list of problems to filter is provided to the task or workspace and the problems will not be reported (in the xml results or problem markers respectively).

The Use Scan task does not support problem filters because it does not produce detailed problems the way the analysis task does.  However, it may be possible to filter some 'illegal use' references from the use scan using the same problem filters as used in the analysis task.

This bug only involves problem filtering, for reference filtering support, see bug 271044.  There is also no plans to work on problem filtering support on use scan integration both in the workspace (API Use Scans preference page) and in an ant task, bug 332763.
Comment 2 Curtis Windatt CLA 2011-02-28 12:54:49 EST
There is no work being done on this item for M6.  We may not have time for it in 3.7.
Comment 3 Curtis Windatt CLA 2011-03-07 16:40:50 EST
This is back in the plan for 3.7.

The approach we want is to take existing api_filter property files and apply them to the use scan task by extracting the reference from the analysis problem.

Example use case:
There are several API tools enabled bundles in the workspace.  A baseline preference is set and the API tools builder (equivalent to the analysis task) is reporting several errors. The user recognizes one of the errors as a known issue.  Using the quickfix, the user adds a new problem filter to the filter store (.api_filters file in the project settings).  These filters can be set when running the analysis task to skip know problems.
(NEW) When running the use scan task, the api filters directory can also be set.  The task will look at each filtered problem and extract the reference involved, creating a reference filter store.  As the use scan proceeds, we will filter out any references matching the store.

Implementation:
1) Create a tool (ant task?) that will take the .api_filter files in the projects and put them into the form expected by the analysis task.
2) Create a class that can take all the ApiProblems in an ApiFilterStore and convert them to a list of References and put those References into some sort of filter store.
3) Modify the Use task to support .api_filters from (1) and access the code from (2) to filter use scan results.
Comment 4 Curtis Windatt CLA 2011-03-07 16:46:58 EST
For (1)

Andrew would be the person to talk to about how PDE Build generates the .api_filters zip while building Eclipse.

You can find an example zip on the test results page of an Eclipse build
http://download.eclipse.org/eclipse/downloads/drops/I20110306-2000/testResults.php
http://download.eclipse.org/eclipse/downloads/drops/I20110306-2000/apitools/apifilters-I20110306-2000.zip

Peter P., are your builds currently using .api_filter files for the analysis task?  If so, then this step may be uncecessary as you should already have the filters in the correct format.  If not, we would have to figure out the best way to provide it.  We might be able to provide a hook in PDE Build.
Comment 5 Curtis Windatt CLA 2011-03-07 16:54:27 EST
For (2)

This is the most difficult part of implementing this.  The api_filters follow the stucture of an API problem, so extracting a use reference will take some work.

    <resource path="src/org/eclipse/team/core/synchronize/SyncInfoTree.java" type="org.eclipse.team.core.synchronize.SyncInfoTree">
        <filter id="338792546">
            <message_arguments>
                <message_argument value="org.eclipse.team.core.synchronize.SyncInfoTree"/>
                <message_argument value="createEmptyChangeEvent()"/>
            </message_arguments>
        </filter>
    </resource>

The type will provide some direction, but the important information is held in the message arguments.  However, the arguments will be different depending on the type of problem, which is determined by the id.

Mike should be able to provide more direction here.  For each filter, you will get an IApiProblem.  From that problem you will have to determine the reference (or no reference).
Comment 6 Curtis Windatt CLA 2011-03-07 16:56:26 EST
For (3)

The structure can probably follow the analysis task exactly.  We would add a new optional parameter to the use scan task and fix the doc accordingly.

Whenever possible we should be reusing the existing filtering code.
Comment 7 Michael Rennie CLA 2011-03-08 10:23:17 EST
It would also be very helpful to look at the class ApiProblemFactory, it has many methods that can be used to work with ApiProblems, like decomposing problem ids, etc.
Comment 8 Andrew Niefer CLA 2011-03-09 13:58:53 EST
(In reply to comment #4)
> For (1)
> 
> Andrew would be the person to talk to about how PDE Build generates the
> .api_filters zip while building Eclipse.
> 

PDE/Build does not create the .api_filters zip, this is done by Kim's releng scripts, it is basically just copying all the .api_filter files from all the plugins:

<copy todir="${filter_store}">
  <fileset dir="${buildDirectory}/plugins" includes="**/.settings/.api_filters" />
  <regexpmapper from="^(org.eclipse.*\/)(\.settings\/)(\.api_filters)" to="\1\3" />
</copy>
Comment 9 Curtis Windatt CLA 2011-04-08 14:24:20 EDT
Created attachment 192868 [details]
Work In Progress
Comment 10 Curtis Windatt CLA 2011-04-15 15:59:29 EDT
Created attachment 193404 [details]
WIP

This patch creates reference filters from the message arguments for api compatibility problems.  The filters are keyed in a map by referencing type.  Keying by referenced type would make for faster filtering, but the filter creation would have to generate the fully qualified name.

Still need to add additional checks when filtering to check that the referenced type and method/field name (if available) match.
Comment 11 Curtis Windatt CLA 2011-04-18 14:30:36 EDT
This will not be in a good enough state before Feature Freeze.  I will continue to work on it during the 3.7 end game so that it can be committed once 3.8 builds begin.
Comment 12 Michael Rennie CLA 2012-09-21 14:25:33 EDT
Created attachment 221360 [details]
wip2

This patch is a further work in progress it adds:
1. a new non-resource based filter store that is used to provide filtering for bundle components
2. hooks in the use scan Ant task for specifying a filter store (likely to change)
3. hooks in the use scan requestor to ask for backing filter stores while scanning
4. filtering during the use scan via #3 above

A few questions remain about how we want to pass in the filters to the Ant task.
1. do we want to copy them all to a folder like:
  /filters
    /bundle A
      .api_filters
    /bundle B
      .api_filters
 ...

2. do we want to copy them all to an archive with a folder structure like point 1 above?

If we copy them all to a folder we can pass in the filters just like we do with the analysis Ant task and the existing AntFilterStore will work as-is.
Comment 13 Curtis Windatt CLA 2012-12-20 17:56:38 EST
I have pushed the patch with some changes to a topic branch cwindatt/bug332772_filter_use_scans

http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?h=cwindatt/bug332772_filter_use_scans&id=17e890b63cb6edbae2612ebbeefca79754f141fc

No filter store was being created for bundle components.  Only project components can create the filter store.  I have fixed up the code in FilterStore to correct this.

Next the problem created by the scan does not have a path, whereas the filter problem in the store does.  The problem comparator checks the path skipping the filter.  For now I have commented out the code.

With the changes the filtering appears to be working.  However I still see the problem in the final report so something is still broken.
Comment 14 Michael Rennie CLA 2012-12-21 13:12:28 EST
(In reply to comment #13)
> I have pushed the patch with some changes to a topic branch
> cwindatt/bug332772_filter_use_scans
> 
> http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?h=cwindatt/
> bug332772_filter_use_scans&id=17e890b63cb6edbae2612ebbeefca79754f141fc
> 
> No filter store was being created for bundle components.  Only project
> components can create the filter store.  I have fixed up the code in
> FilterStore to correct this.
> 
> Next the problem created by the scan does not have a path, whereas the
> filter problem in the store does.  The problem comparator checks the path
> skipping the filter.  For now I have commented out the code.
> 
> With the changes the filtering appears to be working.  However I still see
> the problem in the final report so something is still broken.

I pushed a small change that fixes this. We should be returning false in the use requestor when we determine the backing reference has been filtered.

http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?h=cwindatt/bug332772_filter_use_scans
Comment 15 Curtis Windatt CLA 2012-12-21 17:16:59 EST
http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?h=cwindatt/bug332772_filter_use_scans&id=1f1964b2a46feefe7c9d1a2258fdb99661c3149f

In several commits to the branch:

- Fixed api filters initialization to accept filters without arguments (which caused 221877).  I also updated the tests.
- Added new tests for filterstore.  These still need some tweaking because the resource cannot be found as they don't point to a project.
- Added some tracing, fixed up documentation
Comment 16 Curtis Windatt CLA 2012-12-27 15:42:17 EST
http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?h=cwindatt/bug332772_filter_use_scans&id=71ec275011c411baf19a09ed4f5a8a12158687e4

I fixed up the tests which uncovered additional problems in FilterStore.  Also changed BundleComponent to init when its name is requested.

The remaining open question is whether we are going to finish implementing AntFilterStore and add support in the ant tasks.
Comment 17 Michael Rennie CLA 2012-12-28 10:37:21 EST
(In reply to comment #16)

> Also changed BundleComponent to init when its name is requested.
> 

I think we would need to run the performance tests to see if this has any impact - getName is called all over the place in the builder. Peeking at the code I notice that we don't set the name or the symbolic name until we init the bundle component. Perhaps there is some room for improvement here - we always tried to have certain pieces of information you could ask API elements that would not cause them to initialize. Perhaps we could collect the name of a bundle component when we create it - would improve performance in all the cases where we ask a bundle for its name when all we are doing is computing a build scopes (in the Ant tasks and the builder)

> The remaining open question is whether we are going to finish implementing
> AntFilterStore and add support in the ant tasks.

I think we should get that working as well. To my way of thinking the Ant store is going to basically be the standard FilterStore (non-resource based).
Comment 18 Curtis Windatt CLA 2012-12-28 11:20:47 EST
(In reply to comment #17)
> 
> I think we would need to run the performance tests to see if this has any
> impact - getName is called all over the place in the builder. Peeking at the
> code I notice that we don't set the name or the symbolic name until we init
> the bundle component. Perhaps there is some room for improvement here - we
> always tried to have certain pieces of information you could ask API
> elements that would not cause them to initialize. Perhaps we could collect
> the name of a bundle component when we create it - would improve performance
> in all the cases where we ask a bundle for its name when all we are doing is
> computing a build scopes (in the Ant tasks and the builder)

If getName is called all over the place before init() then I would expect to see more NPEs and bad cache results as the name would change from null to a new value.  Also the provisional API for getName() does not say it may return null or that it could change.

All the other methods to get information from the manifest require an init (getVersion, symbolic name, etc.).  Fortunately, getting the bundle description from the state is very fast (creating the bundle description does take a little longer).

> I think we should get that working as well. To my way of thinking the Ant
> store is going to basically be the standard FilterStore (non-resource based).

With the current implementation, both FilterStore and AntFilterStore would be used in parallel.  I agree that it is more likely filters will be delivered in a separate folder than included in the bundles themselves, but I will hold off on spending the time to implement it until I confirm that is the case.
Comment 19 Curtis Windatt CLA 2013-01-02 12:40:11 EST
The topic branch has been merged with master.

Remaining work:
1) Add ant task property to set folder of api_filter files
2) Add UI setting in use scanner to set (1)
Comment 20 Dani Megert CLA 2013-01-03 06:38:43 EST
(In reply to comment #19)
> The topic branch has been merged with master.
> 
> Remaining work:
> 1) Add ant task property to set folder of api_filter files
> 2) Add UI setting in use scanner to set (1)

This lets the builder fail with NPEs (see bug 397363).
Comment 21 Curtis Windatt CLA 2013-01-04 15:10:44 EST
http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=ee03181185cba4e7b29e6655c453e472598eadbf

Support for a separate folder of api filter files has been added to the ant task and the use scan launcher.

Just need to update the documentation.
Comment 23 Markus Keller CLA 2013-01-08 06:24:14 EST
There's a compile warning in the build now:

1. WARNING in /src/org/eclipse/pde/api/tools/model/tests/FilterStoreTests.java
 (at line 108)
BundleComponent component = getComponent();
The value of the local variable component is not used
Comment 24 Dani Megert CLA 2013-01-08 06:43:09 EST
(In reply to comment #23)
> There's a compile warning in the build now:
> 
> 1. WARNING in
> /src/org/eclipse/pde/api/tools/model/tests/FilterStoreTests.java
>  (at line 108)
> BundleComponent component = getComponent();
> The value of the local variable component is not used

Fixed with http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=333a0830130cd96b21eccc0a1b0b8a0ac1836fab