Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

feat(*): add SDK core #1

Merged
merged 26 commits into from
Jun 20, 2022
Merged

feat(*): add SDK core #1

merged 26 commits into from
Jun 20, 2022

Conversation

burntcarrot
Copy link

@burntcarrot burntcarrot commented Jun 6, 2022

  • SDK core
  • Exporting reports
  • Utilities for working with tree-sitter
  • Integration tests for analyzers
  • Processors
  • TOML generation utils
  • Guides

* support staticcheck
* save report to JSON
* generate TOML files for each issue
* use a markdown parser for parsing issue descriptions
@burntcarrot
Copy link
Author

burntcarrot commented Jun 6, 2022

I'm in favor of using JSON for defining issues; instead of using a Markdown parser for dividing a Markdown document into sections.

Using TOML files.

@burntcarrot burntcarrot marked this pull request as draft June 6, 2022 14:24
* integration tests for analyzers
* parse triggers using tree-sitter
* utilities for working with tree-sitter
* staticcheck trigger for tree-sitter
@burntcarrot burntcarrot marked this pull request as ready for review June 7, 2022 05:17
Copy link
Contributor

@sourya-deepsource sourya-deepsource left a comment

Choose a reason for hiding this comment

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

  1. Make the CLI runner more robust (handle stderr, stdout and also allow safe exits in case of non-zero exit codes).
  2. Processor (takes the result from the CLI output, and converts it to our format): The processor should receive an io.Reader interface, and return the report.
  3. Reporter (takes in the report in our format, and returns a io.Writer): Return an io.Writer to Run and Run should write it to the desired location.
  4. Processors:
    a. Do not write processors for specific CLI formats. Add support for the common ones, like pep8, emacs, go vet, go-ruleguard etc.
    b. Processor should be specified in the struct.
  5. Specifying issues:
    a. Use a single issues.toml:
[[issues]]		
code = "U1000"		
description = """		
This is **an** issue.	
"""
[[issues]]
code="compile"
...
  1. Add support for more languages in tree-sitter comment parser.
  2. Reorganize the code.

@burntcarrot burntcarrot marked this pull request as draft June 7, 2022 12:43
burntcarrot and others added 6 commits June 11, 2022 18:29
* add unit tests
* update SDK structure
* clean analyzer fields
* add unix processor
* add utilities
* test TOML generation and verify content
* add workflow for running tests
refactor(*): refactor SDK and add tests
@burntcarrot burntcarrot marked this pull request as ready for review June 12, 2022 12:31
analyzers/testdata/issues.toml Outdated Show resolved Hide resolved
.golangci.yml Outdated Show resolved Hide resolved
analyzers/sdk.go Outdated Show resolved Hide resolved
analyzers/sdk.go Outdated Show resolved Hide resolved
analyzers/sdk.go Outdated Show resolved Hide resolved
analyzers/utils/processors/unix.go Outdated Show resolved Hide resolved
analyzers/utils/processors/unix.go Outdated Show resolved Hide resolved
analyzers/utils/utils.go Outdated Show resolved Hide resolved
analyzers/utils/utils.go Outdated Show resolved Hide resolved
analyzers/utils/utils.go Outdated Show resolved Hide resolved
@sourya-deepsource
Copy link
Contributor

Can you add a .deepsource.toml? Copy from #2.

@burntcarrot burntcarrot marked this pull request as draft June 14, 2022 09:35
* add integration test for CSS using csslint
* cleanup tests
@burntcarrot burntcarrot marked this pull request as ready for review June 14, 2022 11:23
analyzers/processors/regex.go Outdated Show resolved Hide resolved
analyzers/sdk_test.go Outdated Show resolved Hide resolved
analyzers/utils/utils.go Outdated Show resolved Hide resolved
analyzers/sdk_test.go Outdated Show resolved Hide resolved
guides/writing-analyzers.md Show resolved Hide resolved
guides/writing-analyzers.md Outdated Show resolved Hide resolved
Copy link

@vishnu-deepsource vishnu-deepsource left a comment

Choose a reason for hiding this comment

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

@burntcarrot Write unit tests. Since this SDK lot of data wrangling, not writing unit tests will cause a lot of maintainability issues.

analyzers/build/build.go Outdated Show resolved Hide resolved
analyzers/build/build.go Show resolved Hide resolved
analyzers/build/build.go Outdated Show resolved Hide resolved
analyzers/build/build.go Outdated Show resolved Hide resolved
analyzers/types/types.go Outdated Show resolved Hide resolved
analyzers/processors/regex.go Outdated Show resolved Hide resolved
@burntcarrot
Copy link
Author

Added unit tests; ready for review.

Copy link

@vishnu-deepsource vishnu-deepsource left a comment

Choose a reason for hiding this comment

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

I've reviewed half of the code and there is a significant amount of refactoring pending on this. Will review the remaining files once the existing issues have been addressed.

analyzers/build/build.go Show resolved Hide resolved
analyzers/build/build.go Outdated Show resolved Hide resolved
analyzers/analysistest/analysistest.go Show resolved Hide resolved
analyzers/analysistest/analysistest.go Outdated Show resolved Hide resolved
analyzers/analysistest/analysistest.go Outdated Show resolved Hide resolved
analyzers/analysistest/analysistest.go Outdated Show resolved Hide resolved
analyzers/build/build.go Outdated Show resolved Hide resolved
analyzers/build/build.go Outdated Show resolved Hide resolved
analyzers/build/build.go Outdated Show resolved Hide resolved
analyzers/build/build.go Outdated Show resolved Hide resolved
@vishnu-deepsource vishnu-deepsource changed the base branch from master to sdk June 20, 2022 06:57
analyzers/analysistest/analysistest.go Outdated Show resolved Hide resolved
}

// readMarkdown is a helper utility used for parsing and sanitizing markdown content.
func readMarkdown(content string) (string, error) {

Choose a reason for hiding this comment

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

content can be accepted as a byte array..

Comment on lines +93 to +95
} else {
issue.IssueCode = issueCodeGenerator(content)
}

Choose a reason for hiding this comment

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

nitpick: Do we need this else block? Does it improve readability by much? @sourya-deepsource

@vishnu-deepsource vishnu-deepsource merged commit 85b9245 into deepsourcelabs:sdk Jun 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants