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

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

    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Minor Minor
    • analysis-core-plugin
    • Jenkins 2.89.2
      Static Analysis Utilities 1.93
      Warnings Plug-in 4.64
      Windows

      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.

          [JENKINS-48869] CsharpNamespaceDetector does not recognize "namespace" preceded by UTF-8 BOM

          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.

          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.

          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.

          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.

          Ulli Hafner added a comment - - edited

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

          Ulli Hafner added a comment - - edited Thanks! Hmm, the broken method already has a FIXME for the encoding... 

          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.

          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.

          Ulli Hafner added a comment -

          Ulli Hafner added a comment - See  https://github.com/jenkinsci/analysis-model/commit/c5bdf809bc6ea3274ec425efd42c823c7d9b3d1c .

          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.

          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.

          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...

          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...

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

              Created:
              Updated:
              Resolved: