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

CsharpNamespaceDetector does not recognize "namespace" preceded by UTF-8 BOM

    Details

    • Type: Bug
    • Status: Resolved (View Workflow)
    • Priority: Minor
    • Resolution: Fixed
    • Component/s: analysis-core-plugin
    • Labels:
    • Environment:
      Jenkins 2.89.2
      Static Analysis Utilities 1.93
      Warnings Plug-in 4.64
      Windows
    • Similar Issues:

      Description

      If there are some compiler warnings about a C# source file that is encoded in UTF-8 and starts with a Byte Order Mark, and the BOM is immediately followed by "namespace " and the name of the namespace, then the Warnings Plug-in does not recognize the namespace declaration and instead displays the relative directory name.

      If I insert CRLF between the BOM and "namespace", then the Warnings Plug-in correctly displays the name of the namespace. Likewise, the namespace detector works OK if there is a comment with copyright notices at the top of the file.

      Because I cannot find any namespace detection code in the Warnings Plug-in itself, I believe the problem is in the hudson.plugins.analysis.util.CsharpNamespaceDetector class, defined in Static Analysis Utilities aka analysis-core. It decodes the file as UTF-8 and then checks each line against the regexp "^namespace .*$" until it finds a match. Perhaps this can be changed to allow "\uFEFF" at the start of the line, or more generally whitespace.

        Attachments

          Activity

          Hide
          drulli Ulli Hafner added a comment -

          Yes, Yes, Yes.

          It will take a while until 5.0 is ready but I don't want to loose time by working on multiple branches...

          Show
          drulli Ulli Hafner added a comment - Yes, Yes, Yes. It will take a while until 5.0 is ready but I don't want to loose time by working on multiple branches...
          Hide
          kon Kalle Niemitalo added a comment -

          I looked at https://github.com/jenkinsci/analysis-model/commit/b39af5d5d9c48e363cde0bf38fa636cb26f4d168 and the two previous commits. Is it true that:

          • The Warnings plugin at the Jenkins update center is currently version 4.64, which was built from the master branch of the warnings-plugin repository. This branch does not use analysis-model and is not affected by these changes.
          • The 3.0 branch of warnings-plugin uses analysis-model and will be affected when it is eventually released. The plugin version number will be 5.0 or higher.
          • After these changes, AbstractPackageDetector will discard any BOM at the start of each file and use the configured encoding regardless of what kind of BOM was there.
          Show
          kon Kalle Niemitalo added a comment - I looked at https://github.com/jenkinsci/analysis-model/commit/b39af5d5d9c48e363cde0bf38fa636cb26f4d168 and the two previous commits. Is it true that: The Warnings plugin at the Jenkins update center is currently version 4.64, which was built from the master branch of the warnings-plugin repository. This branch does not use analysis-model and is not affected by these changes. The 3.0 branch of warnings-plugin uses analysis-model and will be affected when it is eventually released. The plugin version number will be 5.0 or higher. After these changes, AbstractPackageDetector will discard any BOM at the start of each file and use the configured encoding regardless of what kind of BOM was there.
          Show
          drulli Ulli Hafner added a comment - See  https://github.com/jenkinsci/analysis-model/commit/c5bdf809bc6ea3274ec425efd42c823c7d9b3d1c .
          Hide
          kon Kalle Niemitalo added a comment -

          Hardcoding UTF-8 in CsharpNamespaceDetector is not causing problems for us. Although we have both UTF-8 and Windows-1252 encoded *.cs files in the same C# projects, our namespace names consist of ASCII characters only, so CsharpNamespaceDetector returns the correct results in practice. However, I think we will eventually have to standardize on one charset (presumably UTF-8) so that warnings-plugin can display the source code correctly.

          If you want to improve the handling of charsets regardless, I think it would be best if it treated *.cs files the same way as /codepage (C# Compiler Options): if the file seems to be UTF-8 or UTF-16, then use that, otherwise use the configured charset. I suspect UTF-16 is rare for C# source code kept in Git repositories, though.

          Show
          kon Kalle Niemitalo added a comment - Hardcoding UTF-8 in CsharpNamespaceDetector is not causing problems for us. Although we have both UTF-8 and Windows-1252 encoded *.cs files in the same C# projects, our namespace names consist of ASCII characters only, so CsharpNamespaceDetector returns the correct results in practice. However, I think we will eventually have to standardize on one charset (presumably UTF-8) so that warnings-plugin can display the source code correctly. If you want to improve the handling of charsets regardless, I think it would be best if it treated *.cs files the same way as /codepage (C# Compiler Options) : if the file seems to be UTF-8 or UTF-16, then use that, otherwise use the configured charset. I suspect UTF-16 is rare for C# source code kept in Git repositories, though.
          Hide
          drulli Ulli Hafner added a comment - - edited

          Thanks! Hmm, the broken method already has a FIXME for the encoding... 

          Show
          drulli Ulli Hafner added a comment - - edited Thanks! Hmm, the broken method already has a FIXME for the encoding... 
          Hide
          kon Kalle Niemitalo added a comment -

          In the attached JENKINS-48869.test-case.zip, the warning at Class1.cs:19 correctly shows up in namespace ConsoleApplication1, but the warning at Program.cs:7 incorrectly shows up in namespace src/ConsoleApplication1.

          Show
          kon Kalle Niemitalo added a comment - In the attached JENKINS-48869.test-case.zip , the warning at Class1.cs:19 correctly shows up in namespace ConsoleApplication1, but the warning at Program.cs:7 incorrectly shows up in namespace src/ConsoleApplication1.
          Hide
          drulli Ulli Hafner added a comment -

          Can you please attach such a file (or a small part of it)? Then creating a test case would be quite simple.

          Show
          drulli Ulli Hafner added a comment - Can you please attach such a file (or a small part of it)? Then creating a test case would be quite simple.

            People

            • Assignee:
              drulli Ulli Hafner
              Reporter:
              kon Kalle Niemitalo
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: