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
feat: add support for gradle.lockfile #2759
Conversation
func (a gradleLockAnalyzer) Required(filePath string, _ os.FileInfo) bool { | ||
return strings.HasSuffix(filepath.Base(filePath), fileNameSuffix) |
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.
os.FileInfo
contains a base file name, right?
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.
Yes. But all language analyzers use filepath
(expect go and rust binaries). That is why i also used filepath
.
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.
i meant that we don't neet to call filepath.Base(filePath)
, because os.FileInfo
already has a file name, right?
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.
You are right. Looks like an extra step.
Used os.FileInfo
@@ -28,7 +28,7 @@ func Test_rustBinaryLibraryAnalyzer_Analyze(t *testing.T) { | |||
FilePath: "testdata/executable_rust", | |||
Libraries: []types.Package{ | |||
{Name: "crate_with_features", Version: "0.1.0"}, | |||
{Name: "library_crate", Version: "0.1.0"}, | |||
{Name: "library_crate", Version: "0.1.0", Indirect: true}, |
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.
why did it do in this PR?
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.
Removed
|
||
const ( | ||
version = 1 | ||
fileNameSuffix = "gradle.lockfile" |
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.
I can see on GitHub, that user have lock files with another names. is it ok?
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 is suffix of filename.
If I understand correctly - we need to check only this suffix.
return strings.HasSuffix(filepath.Base(filePath), fileNameSuffix) |
pkg/fanal/types/const.go
Outdated
@@ -23,6 +23,7 @@ const ( | |||
Pnpm = "pnpm" | |||
Jar = "jar" | |||
Pom = "pom" | |||
GradleLock = "gradle-lock" |
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.
I think Gradle is enough. Other languages don't mention if it is a lock file.
GradleLock = "gradle-lock" | |
Gradle = "gradle" |
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.
Done
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 we also add an integration test?
Done! |
Description
add support for gradle.lockfile
Related issues
Related PRs
Checklist