fix concurrent modification exception

[jeremylong]

Supersedes #4923.

@aikebah this is what I was thinking. currently the persistent object store is on the engine - if it was moved to the settings object there would likely be fewer code changes needed. Thoughts on this approach?

[aikebah]

Supersedes #4923.

@aikebah this is what I was thinking. currently the persistent object store is on the engine - if it was moved to the settings object there would likely be fewer code changes needed. Thoughts on this approach?

Your approach of having it in the engine feels like the clean approach to me. Putting it in settings feels like making the settings object cross the boundary implicitly set by its classname.

[aikebah]

The significant amount of additional required engine method parameters could be avoided by allowing the analyzers to store a reference to the engine in its AbstractAnalyzer#prepareAnalyzer(Engine engine) implementation.
If done like that the AbstractAnalyzer#analyzeDependency(Settings settings, Engine engine) and AbstractAnalyzer#analyze(Settings settings, Engine engine) could also have the second argument removed.

That refactoring could be done in a separate and later change. To me it feels a bit weird to first request an analyzer to prepare itself for running on behalf of an engine instance and then requiring the same engine instance to be supplied again on each and every analysis.

[jeremylong]

@aikebah thank you for the feedback on the PR. The abstract suppression analyzer now stores a reference to the engine and I also moved the call in the engine to report the unused suppression rules to a new analyzer - so that it is cleaner and not a weird one off solution.

[aikebah]

@jeremylong LGTM apart from the nitpicking review comments of Checkstyle