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

Experimental API for malysis integration #14

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

OmkarPh
Copy link
Contributor

@OmkarPh OmkarPh commented Jan 20, 2025

Primarily, I've introduced another core.File implementation - readerFile (can be renamed to "artifactFile" / "standaloneFile" / any suggestions ?)
This allows using ArtifactFile to be used with already available plugin APIs

This can be used in malysis in place of static.go#L201 like -

lang, _ := lang.ResolveLanguageFromPath(af.Name)
artifactParser, err := parser.NewParser(lang)
var readerFile core.File = fs.NewFileFromReader(af.Reader, af.Name, false)
parseTree, _ := artifactParser.Parse(ctx, readerFile)

var buf []byte
stripcommentsPlugin := stripcomments.NewStripCommentsPlugin(func(ctx context.Context, scpd *stripcomments.StripCommentsPluginData) error {
  scpd.Reader.Read(buf)
  return nil
})

if slices.Contains(stripcommentsPlugin.SupportedLanguages(), lang.Meta().Code) {
  stripcommentsPlugin.AnalyzeTree(ctx, parseTree)
} else {
  buf, err = io.ReadAll(io.LimitReader(af.Reader, readLimit))
}

Maybe, we can also have a util function for reusability, created at malysis level, which accepts an artifactFile and returns a io.Reader (reader from StripCommentsPluginData or the default one in case of unsupported lang)

Signed-off-by: Omkar Phansopkar <[email protected]>
@OmkarPh OmkarPh requested a review from abhisek January 20, 2025 10:22
fs/filesystem.go Outdated
@@ -33,6 +43,31 @@ func (f *localFile) IsImport() bool {
return f.isImport
}

func NewReaderFile(reader io.Reader, path string, isImport bool) *readerFile {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func NewReaderFile(reader io.Reader, path string, isImport bool) *readerFile {
func NewFileFromReader(reader io.Reader, path string, isImport bool) *readerFile {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a documentation for this function since it is exported.

lang/factory.go Outdated
@@ -22,3 +24,19 @@ func GetLanguage(name string) (core.Language, error) {

return nil, fmt.Errorf("language not found: %s", name)
}

func ResolveLanguage(filePath string) (core.Language, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a documentation for this function since it is exported.

lang/factory.go Outdated
@@ -22,3 +24,19 @@ func GetLanguage(name string) (core.Language, error) {

return nil, fmt.Errorf("language not found: %s", name)
}

func ResolveLanguage(filePath string) (core.Language, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func ResolveLanguage(filePath string) (core.Language, error) {
func ResolveLanguageFromPath(filePath string) (core.Language, error) {

Since this is an exported function and package global (i.e. not within a struct receiver), I think we should name it as explicitly as possible.

@abhisek
Copy link
Member

abhisek commented Jan 20, 2025

@OmkarPh The Code framework related changes look good to me. I think they are generic enough and can be utilized for various use-cases where we want to parse & analyse a single file and not an entire directory / filesystem. Let us go ahead with this change in Code.

lang/factory.go Outdated
@@ -22,3 +24,19 @@ func GetLanguage(name string) (core.Language, error) {

return nil, fmt.Errorf("language not found: %s", name)
}

func ResolveLanguage(filePath string) (core.Language, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a test for this.

@OmkarPh
Copy link
Contributor Author

OmkarPh commented Jan 20, 2025

updated naming and docs

@OmkarPh OmkarPh requested a review from abhisek January 20, 2025 18:11
@abhisek
Copy link
Member

abhisek commented Jan 21, 2025

@OmkarPh Is this ready for review? I see this is still in draft status

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants