-
Notifications
You must be signed in to change notification settings - Fork 46
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
🐛 Don't use full file paths for issues #351
Conversation
Signed-off-by: Emily McMullan <[email protected]>
kind: Class | ||
name: Singleton | ||
- uri: file:///analyzer-lsp/examples/java/src/main/java/com/example/apps/Singleton.java | ||
message: condition entries should evaluate out of order | ||
codeSnip: " 1 package com.example.apps;\n 2 \n 3 import javax.ejb.SessionBean;\n 4 import javax.ejb.Singleton;\n 5 \n 6 @Singleton\n 7 public class Bean implements SessionBean {\n 8 }\n" | ||
lineNumber: 7 | ||
variables: | ||
file: file:///analyzer-lsp/examples/java/src/main/java/com/example/apps/Singleton.java | ||
file: /src/main/java/com/example/apps/Singleton.java |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like we're trimming off the file://
prefix, which we shouldn't be
@@ -195,7 +199,7 @@ func main() { | |||
} | |||
} | |||
|
|||
rulesets := eng.RunRules(ctx, ruleSets, selectors...) | |||
rulesets := eng.RunRules(ctx, ruleSets, locations, selectors...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we make this a variable on the Engine, something like location prefixes?
for i := range locations { | ||
tempFile := uri.File(locations[i]) | ||
if strings.Contains(string(m.FileURI), locations[i]) { | ||
updatedFile = strings.TrimPrefix(string(m.FileURI), string(tempFile)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should break out of the for loop here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can also add the file:// back here, and fix that issue
updatedFile = strings.TrimPrefix(string(m.FileURI), string(tempFile)) | ||
} | ||
} | ||
for k := range m.Variables { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you just do and then not loop through stuff?
if _, ok := m.Vairables["file"]; ok {
m.Variables["file"] = updatedFile
}
This slipped through somehow after merging #351 I should have re-run CI before merging #557 Signed-off-by: Pranav Gaikwad <[email protected]>
This slipped through somehow after merging konveyor#351 I should have re-run CI before merging konveyor#557 Signed-off-by: Pranav Gaikwad <[email protected]>
This slipped through somehow after merging konveyor#351 I should have re-run CI before merging konveyor#557 Signed-off-by: Pranav Gaikwad <[email protected]>
This slipped through somehow after merging konveyor#351 I should have re-run CI before merging konveyor#557 Signed-off-by: Pranav Gaikwad <[email protected]>
Closes Full file paths for dependencies are irrelevant for the user #322