Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-40439

Move parsers into external library

XMLWordPrintable

    • 5.0.0-beta2

      Rather than holding parsers within warnings-plugin, move them to an external library, so that the same parser can be used for multiple plugins.

      There is https://github.com/tomasbjerre/violations-lib that already duplicates some of the parsers (plus might add a few extra). Would you consider merging the parser logic into that library (and moving it into jenkinsci-organization in Github)?

      See https://github.com/jenkinsci/violations-plugin/issues/88#issuecomment-266990121. I would assume some sort of a merge would be nice here.

          [JENKINS-40439] Move parsers into external library

          Ulli Hafner added a comment - - edited

          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.

          Ulli Hafner added a comment - - edited 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.

          Tuukka Mustonen added a comment -

          Well... unfortunately I think this didn't fly. Should probably be closed?

          Tuukka Mustonen added a comment - Well... unfortunately I think this didn't fly. Should probably be closed?

          Ulli Hafner added a comment -

          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?

          Ulli Hafner added a comment - 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?

          Tuukka Mustonen added a comment -

          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

          Tuukka Mustonen added a comment - 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

          Pedro Furlanetto added a comment -

          Is there any update about CodeNarc parser?

          Pedro Furlanetto added a comment - Is there any update about CodeNarc parser?

          Tuukka Mustonen added a comment -

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

          Tuukka Mustonen added a comment - Hmm, this just moved to In Progress. Is there something going on?

          Ulli Hafner added a comment -

          Yes, this is the reason behind the state change

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

          Ulli Hafner added a comment - Yes, this is the reason behind the state change It is part of the analysis-core refactoring, see wiki for details.

          SCM/JIRA link daemon added a comment -

          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]

          SCM/JIRA link daemon added a comment - 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]

          SCM/JIRA link daemon added a comment -

          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

          SCM/JIRA link daemon added a comment - 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

          Ulli Hafner added a comment -

          Released in 5.0.0-beta2.

          Ulli Hafner added a comment - Released in 5.0.0-beta2.

            drulli Ulli Hafner
            tuukkamustonen Tuukka Mustonen
            Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

              Created:
              Updated:
              Resolved: