Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No annotations created under Windows #177

Closed
adangel opened this issue Mar 18, 2023 · 2 comments · Fixed by #178
Closed

No annotations created under Windows #177

adangel opened this issue Mar 18, 2023 · 2 comments · Fixed by #178
Labels
bug Something isn't working
Milestone

Comments

@adangel
Copy link
Member

adangel commented Mar 18, 2023

It seems, that issue #51 reappeared...

Log output:

  src\classes2\AllInOne.cls:2:FieldNamingConventions (Priority: 1):The instance field name 'INSTANCE_FIELD' doesn't match '[a-z][a-zA-Z0-9]*'
  Error: Configurable naming conventions for field declarations. This rule reports variable declarations
  which do not match the regex that applies to their specific kind ---e.g. constants (static final),
  static field, final field. Each regex can be configured through properties.
  
  By default this rule uses the standard Apex naming convention (Camel case).
  
  FieldNamingConventions (Priority: 1, Ruleset: Code Style)
  https://pmd.github.io/pmd-6.55.0/pmd_rules_apex_codestyle.html#fieldnamingconventions

That means, we use backslashes as path separators. That means, in the sarif.json file, there are no valid URIs anymore...

        {
          "ruleId": "FieldNamingConventions",
          "ruleIndex": 0,
          "message": {
            "text": "The instance field name 'INSTANCE_FIELD' doesn't match '[a-z][a-zA-Z0-9]*'"
          },
          "locations": [
            {
              "physicalLocation": {
                "artifactLocation": {
                  "uri": "src\\classes2\\AllInOne.cls"
                },
                "region": {
                  "startLine": 2,
                  "startColumn": 13,
                  "endLine": 2,
                  "endColumn": 26
                }
              }
            }
          ]
        },

I think, these should be forward slashes....

@adangel adangel added the bug Something isn't working label Mar 18, 2023
@oowekyala
Copy link
Member

I think we should probably remove the display name from TextFile and let renderers pick how they want to render the path name...

@adangel
Copy link
Member Author

adangel commented Mar 18, 2023

I've tested now a bit: Since 6.54.0 it doesn't work anymore. There we introduced the new --relative-with param. Before, the paths were rendered always as absolute, now they are rendered as given. As the action only provides relative paths to analyze, now also only relative paths end up in the report.
The action has a special logic to relativize the paths in the report. And it apparently does two things: relativize the paths and convert any windows paths with backslashes into relative uri paths with forward slashes...

So, two things we can do: populate the filelist with absolute paths or have the action deal with relative paths and just convert into forward slashes... (that's what the github logic to display annotation is requiring...)

adangel added a commit to adangel/pmd-github-action that referenced this issue Mar 18, 2023
@adangel adangel added this to the next milestone Mar 18, 2023
adangel added a commit to adangel/pmd-github-action that referenced this issue Mar 19, 2023
adangel added a commit that referenced this issue Mar 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants