That would make sense, yes. Before we can do this step, we need to define a new API for this plugin (or library).

Questions to address:

1. Does the library depend on Jenkins core? Or is it a plain Java library? Currently the parsers implement ExtensionPoint... Also the serialisation is based on Jenkins XStream. Additionally, the library must be part of a plug-in to get the class loading done.
2. How does the object graph of the result look like? I would like to change the current structure of Warning, ParserResult, etc.
3. Is there a way to obtain only the number of warnings rather than the actual warnings?

PS for a bachelor thesis of one of my students I created a prototype module to see what is actually required: https://github.com/jenkinsci/analysis-model

1. No. Just SLF4J https://github.com/tomasbjerre/violations-lib/blob/master/build.gradle#L43 .
2. Its a list of Violations https://github.com/tomasbjerre/violations-lib/blob/master/src/main/java/se/bjurr/violations/lib/model/Violation.java
3. No.

When it comes to defining an API I am pretty much happy with what I have. But I am open for suggestions!

I took a quick look at analysis-model. The amount of dependencies are a show stopper for me. I would rather spend time on making the parsing algorithms use only functionality from the standard library (1.7).

1a. No Jenkins dependencies means a lot of duplication: a wrapper for each violation model class in analysis-core (otherwise serialization/classloading will not work: maybe this works when exporting the violation-lib classes in analysis-core). Reimplementation of extension point pattern (using a registry), etc. It is possible going this way but this will require a lot of extra work (and testing). The question is what you loose if we have a dependency to Jenkins.
1b. No other dependencies means a lot of rework: several parsers require different libraries. What is the reason behind limiting the dependencies to just logging (why is this a show stopper)? Why Java 7 and not 8?
2. Currently this is almost the same in analysis-core (plus a lot of additional utility classes). This is one thing I would change if I could, this model is quite limited when used in conjunction with DB backend, UI filtering and paging.
3. This is something requested by some users, would be nice if it could be integrated somehow if I totally change the underlying model

1a I use the library also in Gradle and Maven plugins. Others may use it in whatever application they want, perhaps another CI server. Having a dependency to Jenins just does not make sense for a library whose only task is to read and parse text-files.
1b
I prefer to make it Java 7 compatible because its still used by alot of people.
Unnecessary libraries on the classpath will be a problem for users of the library. For example, they may already have another version of Apache commons already on the classpath. It may be there because of another dependency or because they need it themselves. They would have to exclude dependencies and sometimes it will be impossible to resolve the situation and find a version that is compatible with all libraries using it.
I have been able to implement alot of parsers this way, the library is very clean, very easy to work with and understand for anyone familiar with Java. I'd like to keep it that way.

2 That to me sounds like something other then reading and parsing text files. Not a problem that should be solved by the library.

I'm not author of anything here, but here's my two cents:

1. I interpret Tomas' statement so that he's not exactly against dependency libraries (that would be weird, as violations-lib is a dependency for other projects).

The problem of conflicting requirements versions between multiple libraries is common. I not user of NodeJS, but as I've understood it solves that by having isolated requirements for each library (and also means lots of "wasted" disk space and potentially higher memory consumption... just assuming). In other languages you typically specify minimum version instead of pinning to an exact one. Of course, newer versions may break compatibility, so it's not fool proof, but you can constraint maximum version also. Would that work as a compromise, tomasbjerre?

I do agree that this library should not depend on jenkins libraries (analysis-core, violations, script-security). In addition to them, there's only commons-digester3 and I don't see it bad (commons and guava libraries are useful, if they cut the amount of code needed, just use them, imo). Other dependencies are test-scope only, and would probably mostly vanish if you get rid of the jenkins-related libraries.

3. Is there a way to obtain only the number of warnings rather than the actual warnings?

Can't you just .size() the results? You have to run the full analysis anyway. Maybe I'm missing the point here...

1. Sort of yes.

If you take Guava for example. I have been using this method alot:
https://google.github.io/guava/releases/19.0/api/docs/com/google/common/base/Objects.html#firstNonNull(T, T)
As you can read there it says "scheduled for removal in June 2016". If the library was using that method then anyone trying to use the library in an application with latest Guava would get ClassNotFoundException if they have latest Guava on classpath, or they would have to downgrade their Guava to the older one used by the library.

Also commons-digester3 (https://repo1.maven.org/maven2/org/apache/commons/commons-digester3/3.2/commons-digester3-3.2.pom) adds cglib, commons-beanutils, commons-logging + whatever libraries they depend on.

A good library does not force the user of the library into using a specific version of a utility library like this. I dont see the need to use anything else but the standard library here. Show me an example and I may change my mind =)

3. Yes, .size() would work.

I think it is possible to remove the Jenkins dependencies, it just will require some additional work: currently the model is part of analysis-core, the parsers are in the warnings plug-in. If we move the parsers to a separate library then the model needs to be duplicated (and the extension point needs to be replaced by a registry) and after the parsing the model needs to be transformed to the Jenkins model.

On the other hand I'm not so dogmatic on using other libraries. (BTW: if you don't like libraries why don't you use java logging and remove slf4j, too?). I think if I can use a library that reduces the amount of code (and effort) I'll use it. Also I don't care much about things that might happen in future - if it really happens that we have a library dependency problem then we still have the possibility to change it (and JENKINS-40439 is exactly such a change request, isn't it?). Moreover, there are some ways to hide dependencies - if required, e.g. I'm using the maven shade plug-in to hide some of the dependencies in my findbugs parser wrapper that are in conflict with Jenkins dependencies.

Several parsers now use digester and I see no real requirement to reimplement all these parsers just to drop the dependency (of course these parsers could use Java 7 classes only, but why should be rewrite working code?). And this would be even more problematic for the FindBugs parser: here the FindBugs team already provides one (which in contrast to your parser also handles some edge cases), why should I implement another one? I would like to spend this effort somewhere else...

(Just a side note to 3.: the reason for this requirement is from some users having files with thousands of warnings. Parsing these files into a List of warnings in memory is quite slow so using size() on this list will not help. But this is actually not a high priority requirement.)

"if you don't like libraries"

I do like libraries. I dont like libraries that polutes my classpath. I dont want to force users of my library into using a specific version of some other library.

"why don't you use java logging and remove slf4j, too?"

I was using slf4j-api, and only the basic functionality. That is very unlikely to casue problems. But on the other hand I only do very little logging so I actually removed slf4j-api too.

The FindBugs report is tricky, yes. If there is a findbugs-parser ready and working using digester then that, to me, is a valid argument for having digester on the classpath. And use shading to avoid classpath collisions.

BTW: who will depend on this library? Is that already clear? It would help to see their requirements... Currently we have the warnings-plugin and the violation-comments-to-github-plugin in Jenkins.

https://github.com/jenkinsci/violation-comments-to-stash-plugin
https://github.com/tomasbjerre/violation-comments-to-github-maven-plugin
https://github.com/tomasbjerre/violation-comments-to-github-gradle-plugin

But I think I've made my points here. Don't have anything more to add to this discussion.

tuukkamustonen: We clearly have very different views on how this should be solved so I don't see this happening. I think what I have in violations-lib is the perfect approach and if you need any parser implemented, just open an issue there! =)

Don't give up just yet - I think it the discussion has been positive and constructive. I feel you both possess great open-minded attitude, it's just natural that you have different matter of taste, but we're getting close!

About duplication:

If we move the parsers to a separate library then the model needs to be duplicated (and the extension point needs to be replaced by a registry) and after the parsing the model needs to be transformed to the Jenkins model.

I don't know the technical details, but I think it's just natural that you need some sort of an adapter that maps each parser into the Jenkins plugin. Yeah, it will add some code (but you shouldn't need to duplicate code). That's the price you must pay for separation of concerns. However, you will collect the benefits later on.

Would it be possible to somehow, dynamically resolve and construct these adapters, so that you wouldn't have to explicitly write each?

About the dependency libraries:

The FindBugs report is tricky, yes. If there is a findbugs-parser ready and working using digester then that, to me, is a valid argument for having digester on the classpath. And use shading to avoid classpath collisions.

Sounds good. I'm sure you'll come to terms about dependency libraries if you work on this: use what you need, omit what you don't. You'll start with what you got (minus the Jenkins dependencies), and if Tomas wants less dependencies, I'm sure he's quick to refactor the code.

Besides, you can still depend on more dependencies in warnings-plugin. It's just violations-lib that should be minimal.

(of course these parsers could use Java 7 classes only, but why should be rewrite working code?

Same here, I believe Tomas is a pretty productive guy, so he'll quickly do this Simplifying and working on technical debt every now and then makes sense (although, I don't think this categorizes as "technical debt", but anyway).

If you got tests there, it's less likely that things will break.

(Just a side note to 3.: the reason for this requirement is from some users having files with thousands of warnings. Parsing these files into a List of warnings in memory is quite slow so using size() on this list will not help. But this is actually not a high priority requirement.)

Tell them they should really fix their code

I know I'm over-simplifying things but you should "just":

1. Tomas copies/merges the parsers from warnings-plugin into violations-lib.
2. Agree/ensure on the parser API in violations-lib.
3. Ulli switches from built-in parsers into violations-lib. Put whatever you need there, we'll have just single parser implementations in the end, which is the goal of this ticket. You can tune things later on, if need be.

Even if steps 2-3 never succeed, there's benefit in step one, as violations-lib will get quite a few parsers that are now missing.

https://github.com/jenkinsci/violation-comments-to-stash-plugin
https://github.com/tomasbjerre/violation-comments-to-github-maven-plugin
https://github.com/tomasbjerre/violation-comments-to-github-gradle-plugin

But these are only projects under your control, here you can manage the library versions. You did mention 'users of the project' in a previous comment, are these potential users or are there already other people using the library?

But I think I've made my points here. Don't have anything more to add to this discussion. We clearly have very different views on how this should be solved so I don't see this happening.

Well, if you are not open to a discussion, than it will be so. When I spend my spare time on a project I want to make sure that it is a good investment that helps to fulfill some real requirements. I won't just change a lot of code just because the design is better or cleaner. This violates the YAGNI and KISS principles I'm following.

Actually a design or architecture is a solution for a given set of requirements. The only real requirement I'm seeing, is to provide a library of parsers that has no Jenkins dependencies. This is something we can work on.

I don't know the technical details, but I think it's just natural that you need some sort of an adapter that maps each parser into the Jenkins plugin. Yeah, it will add some code (but you shouldn't need to duplicate code). That's the price you must pay for separation of concerns. However, you will collect the benefits later on.

Yes, it's no duplication, just a wrapper for each parser and model class. I've done this for the parsers of the violations plugin already quite some time ago, see ViolationsAdapter. (Note that this implementation is working only partly, since the parsers of the violations plug-in are not serializable and will not work on slave builds.)

Would it be possible to somehow, dynamically resolve and construct these adapters, so that you wouldn't have to explicitly write each?

Yes, see ViolationsRegistry.

Sounds good. I'm sure you'll come to terms about dependency libraries if you work on this: use what you need, omit what you don't. You'll start with what you got (minus the Jenkins dependencies), and if Tomas wants less dependencies, I'm sure he's quick to refactor the code.
Besides, you can still depend on more dependencies in warnings-plugin. It's just violations-lib that should be minimal.
If you got tests there, it's less likely that things will break.

Yes, if you want to replace the parsers with something simpler, go ahead I just don't have the time to replace existing and working (and tested) code with something that produces the same results in a different way

Tell them they should really fix their code

I tried but this issue pops up from time to time

I know I'm over-simplifying things but you should "just":
1. Tomas copies/merges the parsers from warnings-plugin into violations-lib.
2. Agree/ensure on the parser API in violations-lib.
3. Ulli switches from built-in parsers into violations-lib. Put whatever you need there, we'll have just single parser implementations in the end, which is the goal of this ticket. You can tune things later on, if need be.
Even if steps 2-3 never succeed, there's benefit in step one, as violations-lib will get quite a few parsers that are now missing.

I think this is a feasible approach. I think 2 should be done before 1: otherwise a lot of things will break afterwards.

Please let it open. Maybe I can at least integrate some of the parsers into the warnings plug-in in the same style I am using the parsers of the violations plug-in.

Which parsers are actually missing that should be recognized by Jenkins?

Well, Tomas just added mypy (https://github.com/tomasbjerre/violations-lib/issues/13) and pydocstyle (https://github.com/tomasbjerre/violations-lib/issues/12) that are not available in warnings.

I have been using custom parsers (via Manage jenkins > Configure system) for those with warnings plugin so I haven't ended up submitting anything... would be nice to have them directly in warnings plugin, of course.

I am personally not missing anything else

Is there any update about CodeNarc parser?

Hmm, this just moved to In Progress. Is there something going on?

Yes, this is the reason behind the state change

It is part of the analysis-core refactoring, see wiki for details.

Code changed in jenkins
User: Ulli Hafner
Path:
pom.xml
src/main/java/edu/hm/hafner/analysis/parser/ResharperInspectCodeParser.java
src/main/java/edu/hm/hafner/analysis/parser/violations/AbstractViolationAdapter.java
src/main/java/edu/hm/hafner/analysis/parser/violations/AndroidLintParserAdapter.java
src/main/java/edu/hm/hafner/analysis/parser/violations/CodeNarcAdapter.java
src/main/java/edu/hm/hafner/analysis/parser/violations/CppCheckAdapter.java
src/main/java/edu/hm/hafner/analysis/parser/violations/DocFxAdapter.java
src/main/java/edu/hm/hafner/analysis/parser/violations/ErrorProneAdapter.java
src/main/java/edu/hm/hafner/analysis/parser/violations/Flake8Adapter.java
src/main/java/edu/hm/hafner/analysis/parser/violations/JsHintAdapter.java
src/main/java/edu/hm/hafner/analysis/parser/violations/KlocWorkAdapter.java
src/main/java/edu/hm/hafner/analysis/parser/violations/MyPyAdapter.java
src/main/java/edu/hm/hafner/analysis/parser/violations/PitAdapter.java
src/main/java/edu/hm/hafner/analysis/parser/violations/PyDocStyleAdapter.java
src/main/java/edu/hm/hafner/analysis/parser/violations/ResharperInspectCodeAdapter.java
src/main/java/edu/hm/hafner/analysis/parser/violations/XmlLintAdapter.java
src/main/java/edu/hm/hafner/analysis/parser/violations/ZptLintAdapter.java
src/test/java/edu/hm/hafner/analysis/parser/ResharperInspectCodeParserTest.java
src/test/java/edu/hm/hafner/analysis/parser/violations/AndroidLintParserAdapterTest.java
src/test/java/edu/hm/hafner/analysis/parser/violations/CodeNarcAdapterTest.java
src/test/java/edu/hm/hafner/analysis/parser/violations/CppCheckAdapterTest.java
src/test/java/edu/hm/hafner/analysis/parser/violations/DocFxAdapterTest.java
src/test/java/edu/hm/hafner/analysis/parser/violations/ErrorProneAdapterTest.java
src/test/java/edu/hm/hafner/analysis/parser/violations/Flake8AdapterTest.java
src/test/java/edu/hm/hafner/analysis/parser/violations/JsHintAdapterTest.java
src/test/java/edu/hm/hafner/analysis/parser/violations/KlocWorkAdapterTest.java
src/test/java/edu/hm/hafner/analysis/parser/violations/MyPyAdapterTest.java
src/test/java/edu/hm/hafner/analysis/parser/violations/PitAdapterTest.java
src/test/java/edu/hm/hafner/analysis/parser/violations/PyDocStyleAdapterTest.java
src/test/java/edu/hm/hafner/analysis/parser/violations/ResharperInspectCodeAdapterTest.java
src/test/java/edu/hm/hafner/analysis/parser/violations/XmlLintAdapterTest.java
src/test/java/edu/hm/hafner/analysis/parser/violations/ZptLintAdapterTest.java
src/test/resources/edu/hm/hafner/analysis/parser/ResharperInspectCode.xml
src/test/resources/edu/hm/hafner/analysis/parser/codenarc/CodeNarcReport2.xml
src/test/resources/edu/hm/hafner/analysis/parser/codenarc/CodeNarcXmlReport.xml
src/test/resources/edu/hm/hafner/analysis/parser/findbugs/spotbugsXml.xml
src/test/resources/edu/hm/hafner/analysis/parser/violations/ResharperInspectCode.xml
src/test/resources/edu/hm/hafner/analysis/parser/violations/android-lint.xml
src/test/resources/edu/hm/hafner/analysis/parser/violations/codeNarc.xml
src/test/resources/edu/hm/hafner/analysis/parser/violations/cppcheck.xml
src/test/resources/edu/hm/hafner/analysis/parser/violations/docfx.json
src/test/resources/edu/hm/hafner/analysis/parser/violations/error-prone.log
src/test/resources/edu/hm/hafner/analysis/parser/violations/flake8.txt
src/test/resources/edu/hm/hafner/analysis/parser/violations/jshint.xml
src/test/resources/edu/hm/hafner/analysis/parser/violations/klocwork.xml
src/test/resources/edu/hm/hafner/analysis/parser/violations/mypy.txt
src/test/resources/edu/hm/hafner/analysis/parser/violations/pit.xml
src/test/resources/edu/hm/hafner/analysis/parser/violations/pydocstyle.txt
src/test/resources/edu/hm/hafner/analysis/parser/violations/xmllint.txt
src/test/resources/edu/hm/hafner/analysis/parser/violations/zptlint.log
http://jenkins-ci.org/commit/analysis-model/f22e29dd33c93cdae9fd228bbce7bff9a2eac517
Log:
Let all parsers not depend on Jenkins anymore.

Integrated all parsers into new module analysis-model. Added adapter
classes for parsers of violations-lib.

• [FIXED JENKINS-17434]
• [FIXED JENKINS-40439]

Code changed in jenkins
User: Ulli Hafner
Path:
src/main/java/io/jenkins/plugins/analysis/warnings/AndroidLint.java
src/main/java/io/jenkins/plugins/analysis/warnings/CodeNArc.java
src/main/java/io/jenkins/plugins/analysis/warnings/CppCheck.java
src/main/java/io/jenkins/plugins/analysis/warnings/DocFx.java
src/main/java/io/jenkins/plugins/analysis/warnings/ErrorProne.java
src/main/java/io/jenkins/plugins/analysis/warnings/Flake8.java
src/main/java/io/jenkins/plugins/analysis/warnings/JsHint.java
src/main/java/io/jenkins/plugins/analysis/warnings/KlocWork.java
src/main/java/io/jenkins/plugins/analysis/warnings/MyPy.java
src/main/java/io/jenkins/plugins/analysis/warnings/Pit.java
src/main/java/io/jenkins/plugins/analysis/warnings/PyDocStyle.java
src/main/java/io/jenkins/plugins/analysis/warnings/ResharperInspectCode.java
src/main/java/io/jenkins/plugins/analysis/warnings/Xlc.java
src/main/java/io/jenkins/plugins/analysis/warnings/XmlLint.java
src/main/java/io/jenkins/plugins/analysis/warnings/ZptLint.java
src/main/resources/io/jenkins/plugins/analysis/warnings/Messages.properties
src/test/java/hudson/plugins/warnings/parser/ResharperInspectCodeAdapterTest.java
src/test/java/hudson/plugins/warnings/parser/ResharperInspectCodeParserTest.java
src/test/java/io/jenkins/plugins/analysis/warnings/ParsersITest.java
src/test/resources/io/jenkins/plugins/analysis/warnings/android-lint.xml
src/test/resources/io/jenkins/plugins/analysis/warnings/codeNarc.xml
src/test/resources/io/jenkins/plugins/analysis/warnings/cppcheck.xml
src/test/resources/io/jenkins/plugins/analysis/warnings/docfx.json
src/test/resources/io/jenkins/plugins/analysis/warnings/error-prone.log
src/test/resources/io/jenkins/plugins/analysis/warnings/flake8.txt
src/test/resources/io/jenkins/plugins/analysis/warnings/jshint.xml
src/test/resources/io/jenkins/plugins/analysis/warnings/klocwork.xml
src/test/resources/io/jenkins/plugins/analysis/warnings/mypy.txt
src/test/resources/io/jenkins/plugins/analysis/warnings/pit.xml
src/test/resources/io/jenkins/plugins/analysis/warnings/pydocstyle.txt
src/test/resources/io/jenkins/plugins/analysis/warnings/spotbugsXml.xml
src/test/resources/io/jenkins/plugins/analysis/warnings/xmllint.txt
src/test/resources/io/jenkins/plugins/analysis/warnings/zptlint.log
http://jenkins-ci.org/commit/warnings-plugin/dcbd8e1b3b660ffa3764486557491849e77d63d9
Log:
Let all parsers not depend on Jenkins anymore.

Integrated all parsers into new module analysis-model. Added adapter
classes for parsers of violations-lib.

• [FIXED JENKINS-17434]
• [FIXED JENKINS-40439]

Compare: https://github.com/jenkinsci/warnings-plugin/compare/3f29bdbcfac9...dcbd8e1b3b66

Released in 5.0.0-beta2.