From 4f98ea0ee7d4279f547bae760392d213669a2348 Mon Sep 17 00:00:00 2001 From: Ryan Mast Date: Thu, 14 May 2020 18:08:11 -0700 Subject: [PATCH 1/7] Work-in progress code-scanning --- github/authorizations.go | 1 + github/code-scanning.go | 95 ++++++++++++++++++++++++++++++++++++++++ github/github.go | 2 + 3 files changed, 98 insertions(+) create mode 100644 github/code-scanning.go diff --git a/github/authorizations.go b/github/authorizations.go index 7447e2fbf7..ad31ebf208 100644 --- a/github/authorizations.go +++ b/github/authorizations.go @@ -41,6 +41,7 @@ const ( ScopeReadGPGKey Scope = "read:gpg_key" ScopeWriteGPGKey Scope = "write:gpg_key" ScopeAdminGPGKey Scope = "admin:gpg_key" + ScopeSecurityEvents Scope = "security_events" ) // AuthorizationsService handles communication with the authorization related diff --git a/github/code-scanning.go b/github/code-scanning.go new file mode 100644 index 0000000000..bb3a414ae1 --- /dev/null +++ b/github/code-scanning.go @@ -0,0 +1,95 @@ +// Copyright 2020 The go-github AUTHORS. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package github + +import ( + "context" + "time" +) + +// CodeScanningService handles communication with the code scanning related +// methods of the GitHub API. +// +// GitHub API docs: https://developer.github.com/v3/code-scanning/ +type CodeScanningService service + +type Alert struct { + RuleID *string `json:"rule_id,omitempty"` + RuleSeverity *string `json:"rule_severity,omitempty"` + RuleDescription *string `json:"rule_description,omitempty"` + Tool *string `json:"tool,omitempty"` + CreatedAt *time.Time `json:"created_at,omitempty"` + Open *bool `json:"open,omitempty"` + ClosedBy *string `json:"closed_by,omitempty"` + ClosedAt *time.Time `json:"closed_at,omitempty"` + URL *string `json:"url,omitempty"` + HTMLURL *string `json:"html_url,omitempty"` +} + +// AlertListOptions specifies optional parameters to the CodeScanningService.ListAlerts +// method. +type AlertListOptions struct { + // State of the code scanning alerts to list. Set to closed to list only closed code scanning alerts. Default: open + State string `url:"state,omitempty"` + + // Return code scanning alerts for a specific branch reference. The ref must be formatted as heads/. + Ref string `url:"ref,omitempty"` +} + +// ListAlerts lists code scanning alerts for a repository. +// +// Lists all open code scanning alerts for the default branch (usually master) and protected branches in a repository. +// You must use an access token with the security_events scope to use this endpoint. GitHub Apps must have the security_events +// read permission to use this endpoint. +// +// state: Set to closed to list only closed code scanning alerts. Default: open +// ref: Returns a list of code scanning alerts for a specific branch reference. The ref must be formatted as heads/. +// GitHub API docs: https://developer.github.com/v3/code-scanning/#list-code-scanning-alerts-for-a-repository +func (s *CodeScanningService) ListAlerts(ctx context.Context, owner, repo string, opts *AlertListOptions) ([]*Alert, *Response, error) { + u := fmt.Sprintf("repos/%v/%v/code-scanning/alerts", owner, repo) + u, err := addOptions(u, opts) + if err != nil { + return nil, nil, err + } + + req, err := s.client.NewRequest("GET", u, nil) + if err != nil { + return nil, nil, err + } + + var alerts []*Alert + resp, err := s.client.Do(ctx, req, &alerts) + if err != nil { + return nil, resp, err + } + + return alerts, resp, nil +} + +// GetAlert gets a single code scanning alert for a repository. +// +// You must use an access token with the security_events scope to use this endpoint. +// GitHub Apps must have the security_events read permission to use this endpoint. +// +// The security alert_id is the number at the end of the security alert's URL. +// +// GitHub API docs: https://developer.github.com/v3/code-scanning/#get-a-code-scanning-alert +func (s *CodeScanningService) GetAlert(ctx context.Context, owner, repo string, id int64) (*Alert, *Response, error) { + u := fmt.Sprintf("repos/%v/%v/code-scanning/alerts/%v", owner, repo, id) + + req, err := s.client.NewRequest("GET", u, nil) + if err != nil { + return nil, nil, err + } + + a := new(RegistrationToken) + resp, err := s.client.Do(ctx, req, a) + if err != nil { + return nil, resp, err + } + + return a, resp, nil +} diff --git a/github/github.go b/github/github.go index 888da21274..d876e98bbd 100644 --- a/github/github.go +++ b/github/github.go @@ -165,6 +165,7 @@ type Client struct { Apps *AppsService Authorizations *AuthorizationsService Checks *ChecksService + CodeScanning *CodeScanningService Gists *GistsService Git *GitService Gitignores *GitignoresService @@ -271,6 +272,7 @@ func NewClient(httpClient *http.Client) *Client { c.Apps = (*AppsService)(&c.common) c.Authorizations = (*AuthorizationsService)(&c.common) c.Checks = (*ChecksService)(&c.common) + c.CodeScanning = (*CodeScanningService)(&c.common) c.Gists = (*GistsService)(&c.common) c.Git = (*GitService)(&c.common) c.Gitignores = (*GitignoresService)(&c.common) From 6051648c5cc5c27dad78d5d82ff6671116efd060 Mon Sep 17 00:00:00 2001 From: Ryan Mast Date: Sat, 30 May 2020 10:36:43 -0700 Subject: [PATCH 2/7] Add code scanning tests and fix bugs --- github/code-scanning.go | 29 ++++---- github/code-scanning_test.go | 127 +++++++++++++++++++++++++++++++++++ github/github-accessors.go | 80 ++++++++++++++++++++++ 3 files changed, 221 insertions(+), 15 deletions(-) create mode 100644 github/code-scanning_test.go diff --git a/github/code-scanning.go b/github/code-scanning.go index bb3a414ae1..1e2c2c3436 100644 --- a/github/code-scanning.go +++ b/github/code-scanning.go @@ -7,6 +7,7 @@ package github import ( "context" + "fmt" "time" ) @@ -17,16 +18,16 @@ import ( type CodeScanningService service type Alert struct { - RuleID *string `json:"rule_id,omitempty"` - RuleSeverity *string `json:"rule_severity,omitempty"` - RuleDescription *string `json:"rule_description,omitempty"` - Tool *string `json:"tool,omitempty"` - CreatedAt *time.Time `json:"created_at,omitempty"` - Open *bool `json:"open,omitempty"` - ClosedBy *string `json:"closed_by,omitempty"` - ClosedAt *time.Time `json:"closed_at,omitempty"` - URL *string `json:"url,omitempty"` - HTMLURL *string `json:"html_url,omitempty"` + RuleID *string `json:"rule_id,omitempty"` + RuleSeverity *string `json:"rule_severity,omitempty"` + RuleDescription *string `json:"rule_description,omitempty"` + Tool *string `json:"tool,omitempty"` + CreatedAt *time.Time `json:"created_at,omitempty"` + Open *bool `json:"open,omitempty"` + ClosedBy *string `json:"closed_by,omitempty"` + ClosedAt *time.Time `json:"closed_at,omitempty"` + URL *string `json:"url,omitempty"` + HTMLURL *string `json:"html_url,omitempty"` } // AlertListOptions specifies optional parameters to the CodeScanningService.ListAlerts @@ -39,16 +40,14 @@ type AlertListOptions struct { Ref string `url:"ref,omitempty"` } -// ListAlerts lists code scanning alerts for a repository. +// ListAlertsForRepo lists code scanning alerts for a repository. // // Lists all open code scanning alerts for the default branch (usually master) and protected branches in a repository. // You must use an access token with the security_events scope to use this endpoint. GitHub Apps must have the security_events // read permission to use this endpoint. // -// state: Set to closed to list only closed code scanning alerts. Default: open -// ref: Returns a list of code scanning alerts for a specific branch reference. The ref must be formatted as heads/. // GitHub API docs: https://developer.github.com/v3/code-scanning/#list-code-scanning-alerts-for-a-repository -func (s *CodeScanningService) ListAlerts(ctx context.Context, owner, repo string, opts *AlertListOptions) ([]*Alert, *Response, error) { +func (s *CodeScanningService) ListAlertsForRepo(ctx context.Context, owner, repo string, opts *AlertListOptions) ([]*Alert, *Response, error) { u := fmt.Sprintf("repos/%v/%v/code-scanning/alerts", owner, repo) u, err := addOptions(u, opts) if err != nil { @@ -85,7 +84,7 @@ func (s *CodeScanningService) GetAlert(ctx context.Context, owner, repo string, return nil, nil, err } - a := new(RegistrationToken) + a := new(Alert) resp, err := s.client.Do(ctx, req, a) if err != nil { return nil, resp, err diff --git a/github/code-scanning_test.go b/github/code-scanning_test.go new file mode 100644 index 0000000000..a8181d4c50 --- /dev/null +++ b/github/code-scanning_test.go @@ -0,0 +1,127 @@ +// Copyright 2020 The go-github AUTHORS. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package github + +import ( + "context" + "fmt" + "net/http" + "reflect" + "testing" + "time" +) + +func TestActionsService_ListAlertsForRepo(t *testing.T) { + client, mux, _, teardown := setup() + defer teardown() + + mux.HandleFunc("/repos/o/r/code-scanning/alerts", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + testFormValues(t, r, values{"state": "open", "ref": "heads/master"}) + fmt.Fprint(w, `[{ + "rule_id":"js/trivial-conditional", + "rule_severity":"warning", + "rule_description":"Useless conditional", + "tool":"CodeQL", + "created_at":"2020-05-06T12:00:00Z", + "open":true, + "closed_by":null, + "closed_at":null, + "url":"https://api.github.com/repos/o/r/code-scanning/alerts/25", + "html_url":"https://github.com/o/r/security/code-scanning/25" + }, + { + "rule_id":"js/useless-expression", + "rule_severity":"warning", + "rule_description":"Expression has no effect", + "tool":"CodeQL", + "created_at":"2020-05-06T12:00:00Z", + "open":true, + "closed_by":null, + "closed_at":null, + "url":"https://api.github.com/repos/o/r/code-scanning/alerts/88", + "html_url":"https://github.com/o/r/security/code-scanning/88" + }]`) + }) + + opts := &AlertListOptions{State: "open", Ref: "heads/master"} + alerts, _, err := client.CodeScanning.ListAlertsForRepo(context.Background(), "o", "r", opts) + if err != nil { + t.Errorf("CodeScanning.ListAlertsForRepo returned error: %v", err) + } + + date := time.Date(2020, time.May, 06, 12, 00, 00, 0, time.UTC) + want := []*Alert{ + { + RuleID: String("js/trivial-conditional"), + RuleSeverity: String("warning"), + RuleDescription: String("Useless conditional"), + Tool: String("CodeQL"), + CreatedAt: &date, + Open: Bool(true), + ClosedBy: nil, + ClosedAt: nil, + URL: String("https://api.github.com/repos/o/r/code-scanning/alerts/25"), + HTMLURL: String("https://github.com/o/r/security/code-scanning/25"), + }, + { + RuleID: String("js/useless-expression"), + RuleSeverity: String("warning"), + RuleDescription: String("Expression has no effect"), + Tool: String("CodeQL"), + CreatedAt: &date, + Open: Bool(true), + ClosedBy: nil, + ClosedAt: nil, + URL: String("https://api.github.com/repos/o/r/code-scanning/alerts/88"), + HTMLURL: String("https://github.com/o/r/security/code-scanning/88"), + }, + } + if !reflect.DeepEqual(alerts, want) { + t.Errorf("CodeScanning.ListAlertsForRepo returned %+v, want %+v", alerts, want) + } +} + +func TestActionsService_GetAlert(t *testing.T) { + client, mux, _, teardown := setup() + defer teardown() + + mux.HandleFunc("/repos/o/r/code-scanning/alerts/88", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + fmt.Fprint(w, `{"rule_id":"js/useless-expression", + "rule_severity":"warning", + "rule_description":"Expression has no effect", + "tool":"CodeQL", + "created_at":"2019-01-02T15:04:05Z", + "open":true, + "closed_by":null, + "closed_at":null, + "url":"https://api.github.com/repos/o/r/code-scanning/alerts/88", + "html_url":"https://github.com/o/r/security/code-scanning/88"}`) + }) + + alert, _, err := client.CodeScanning.GetAlert(context.Background(), "o", "r", 88) + if err != nil { + t.Errorf("CodeScanning.GetAlert returned error: %v", err) + } + + date := time.Date(2019, time.January, 02, 15, 04, 05, 0, time.UTC) + want := &Alert{ + RuleID: String("js/useless-expression"), + RuleSeverity: String("warning"), + RuleDescription: String("Expression has no effect"), + Tool: String("CodeQL"), + CreatedAt: &date, + Open: Bool(true), + ClosedBy: nil, + ClosedAt: nil, + URL: String("https://api.github.com/repos/o/r/code-scanning/alerts/88"), + HTMLURL: String("https://github.com/o/r/security/code-scanning/88"), + } + if !reflect.DeepEqual(alert, want) { + t.Errorf("CodeScanning.GetAlert returned %+v, want %+v", alert, want) + } +} diff --git a/github/github-accessors.go b/github/github-accessors.go index 4489e4dcad..b55fbf7bc8 100644 --- a/github/github-accessors.go +++ b/github/github-accessors.go @@ -108,6 +108,86 @@ func (a *AdminStats) GetUsers() *UserStats { return a.Users } +// GetClosedAt returns the ClosedAt field if it's non-nil, zero value otherwise. +func (a *Alert) GetClosedAt() time.Time { + if a == nil || a.ClosedAt == nil { + return time.Time{} + } + return *a.ClosedAt +} + +// GetClosedBy returns the ClosedBy field if it's non-nil, zero value otherwise. +func (a *Alert) GetClosedBy() string { + if a == nil || a.ClosedBy == nil { + return "" + } + return *a.ClosedBy +} + +// GetCreatedAt returns the CreatedAt field if it's non-nil, zero value otherwise. +func (a *Alert) GetCreatedAt() time.Time { + if a == nil || a.CreatedAt == nil { + return time.Time{} + } + return *a.CreatedAt +} + +// GetHTMLURL returns the HTMLURL field if it's non-nil, zero value otherwise. +func (a *Alert) GetHTMLURL() string { + if a == nil || a.HTMLURL == nil { + return "" + } + return *a.HTMLURL +} + +// GetOpen returns the Open field if it's non-nil, zero value otherwise. +func (a *Alert) GetOpen() bool { + if a == nil || a.Open == nil { + return false + } + return *a.Open +} + +// GetRuleDescription returns the RuleDescription field if it's non-nil, zero value otherwise. +func (a *Alert) GetRuleDescription() string { + if a == nil || a.RuleDescription == nil { + return "" + } + return *a.RuleDescription +} + +// GetRuleID returns the RuleID field if it's non-nil, zero value otherwise. +func (a *Alert) GetRuleID() string { + if a == nil || a.RuleID == nil { + return "" + } + return *a.RuleID +} + +// GetRuleSeverity returns the RuleSeverity field if it's non-nil, zero value otherwise. +func (a *Alert) GetRuleSeverity() string { + if a == nil || a.RuleSeverity == nil { + return "" + } + return *a.RuleSeverity +} + +// GetTool returns the Tool field if it's non-nil, zero value otherwise. +func (a *Alert) GetTool() string { + if a == nil || a.Tool == nil { + return "" + } + return *a.Tool +} + +// GetURL returns the URL field if it's non-nil, zero value otherwise. +func (a *Alert) GetURL() string { + if a == nil || a.URL == nil { + return "" + } + return *a.URL +} + // GetVerifiablePasswordAuthentication returns the VerifiablePasswordAuthentication field if it's non-nil, zero value otherwise. func (a *APIMeta) GetVerifiablePasswordAuthentication() bool { if a == nil || a.VerifiablePasswordAuthentication == nil { From 8278503cf4ecb5363f187713c0e6926fda595440 Mon Sep 17 00:00:00 2001 From: Ryan Mast Date: Sat, 30 May 2020 14:48:24 -0700 Subject: [PATCH 3/7] Replace time.Time with Timestamp --- github/code-scanning.go | 5 ++--- github/code-scanning_test.go | 4 ++-- github/github-accessors.go | 8 ++++---- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/github/code-scanning.go b/github/code-scanning.go index 1e2c2c3436..607809b9b3 100644 --- a/github/code-scanning.go +++ b/github/code-scanning.go @@ -8,7 +8,6 @@ package github import ( "context" "fmt" - "time" ) // CodeScanningService handles communication with the code scanning related @@ -22,10 +21,10 @@ type Alert struct { RuleSeverity *string `json:"rule_severity,omitempty"` RuleDescription *string `json:"rule_description,omitempty"` Tool *string `json:"tool,omitempty"` - CreatedAt *time.Time `json:"created_at,omitempty"` + CreatedAt *Timestamp `json:"created_at,omitempty"` Open *bool `json:"open,omitempty"` ClosedBy *string `json:"closed_by,omitempty"` - ClosedAt *time.Time `json:"closed_at,omitempty"` + ClosedAt *Timestamp `json:"closed_at,omitempty"` URL *string `json:"url,omitempty"` HTMLURL *string `json:"html_url,omitempty"` } diff --git a/github/code-scanning_test.go b/github/code-scanning_test.go index a8181d4c50..d7ae3d98ec 100644 --- a/github/code-scanning_test.go +++ b/github/code-scanning_test.go @@ -53,7 +53,7 @@ func TestActionsService_ListAlertsForRepo(t *testing.T) { t.Errorf("CodeScanning.ListAlertsForRepo returned error: %v", err) } - date := time.Date(2020, time.May, 06, 12, 00, 00, 0, time.UTC) + date := Timestamp{time.Date(2020, time.May, 06, 12, 00, 00, 0, time.UTC)} want := []*Alert{ { RuleID: String("js/trivial-conditional"), @@ -108,7 +108,7 @@ func TestActionsService_GetAlert(t *testing.T) { t.Errorf("CodeScanning.GetAlert returned error: %v", err) } - date := time.Date(2019, time.January, 02, 15, 04, 05, 0, time.UTC) + date := Timestamp{time.Date(2019, time.January, 02, 15, 04, 05, 0, time.UTC)} want := &Alert{ RuleID: String("js/useless-expression"), RuleSeverity: String("warning"), diff --git a/github/github-accessors.go b/github/github-accessors.go index b55fbf7bc8..2fa325ddbc 100644 --- a/github/github-accessors.go +++ b/github/github-accessors.go @@ -109,9 +109,9 @@ func (a *AdminStats) GetUsers() *UserStats { } // GetClosedAt returns the ClosedAt field if it's non-nil, zero value otherwise. -func (a *Alert) GetClosedAt() time.Time { +func (a *Alert) GetClosedAt() Timestamp { if a == nil || a.ClosedAt == nil { - return time.Time{} + return Timestamp{} } return *a.ClosedAt } @@ -125,9 +125,9 @@ func (a *Alert) GetClosedBy() string { } // GetCreatedAt returns the CreatedAt field if it's non-nil, zero value otherwise. -func (a *Alert) GetCreatedAt() time.Time { +func (a *Alert) GetCreatedAt() Timestamp { if a == nil || a.CreatedAt == nil { - return time.Time{} + return Timestamp{} } return *a.CreatedAt } From 0d7062fc6b3a15abe69098fe7ce5f206bf89b98c Mon Sep 17 00:00:00 2001 From: Ryan Mast Date: Sat, 30 May 2020 16:32:06 -0700 Subject: [PATCH 4/7] Add helper to get an Alert ID from one of the URLs --- github/code-scanning.go | 41 +++++++++++++++++++++++++++++ github/code-scanning_test.go | 50 ++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/github/code-scanning.go b/github/code-scanning.go index 607809b9b3..4bf8ea399c 100644 --- a/github/code-scanning.go +++ b/github/code-scanning.go @@ -8,6 +8,8 @@ package github import ( "context" "fmt" + "strconv" + "strings" ) // CodeScanningService handles communication with the code scanning related @@ -29,6 +31,45 @@ type Alert struct { HTMLURL *string `json:"html_url,omitempty"` } +// ID() returns the ID associated with an alert. It is the number at the end of the security alert's URL. +func (a *Alert) ID() int64 { + if a == nil { + return 0 + } + + // Set the url to check to "" and index of last forward slash to -1 + s := string("") + i := int(-1) + + // Check if HTMLURL is valid + if a.HTMLURL != nil { + s = *a.HTMLURL + } + + // Check HTMLURL for an ID to parse first, since it was used in the example + if i = strings.LastIndex(s, "/"); i != -1 { + s = s[i+1:] + } else { + // HTMLURL had no forward slashes, try URL as a fallback + // Check if URL is valid + if a.URL != nil { + s = *a.URL + } + if i = strings.LastIndex(s, "/"); i != -1 { + s = s[i+1:] + } else { + // Both HTMLURL and URL were invalid and had no ID + return 0 + } + } + + // Return the alert ID as a 64-bit integer. Unable to convert or out of range returns 0. + if id, err := strconv.ParseInt(s, 10, 64); err == nil { + return id + } + return 0 +} + // AlertListOptions specifies optional parameters to the CodeScanningService.ListAlerts // method. type AlertListOptions struct { diff --git a/github/code-scanning_test.go b/github/code-scanning_test.go index d7ae3d98ec..9fc4206f5b 100644 --- a/github/code-scanning_test.go +++ b/github/code-scanning_test.go @@ -14,6 +14,56 @@ import ( "time" ) +func TestActionsService_Alert_ID(t *testing.T) { + // Test: nil Alert ID == 0 + a := (*Alert)(nil) + id := a.ID() + want := int64(0) + if id != want { + t.Errorf("Alert.ID error returned %+v, want %+v", id, want) + } + + // Test: Valid HTMLURL + a = &Alert{ + HTMLURL: String("https://github.com/o/r/security/code-scanning/88"), + URL: String("https://github.com/repos/o/r/code-scanning/alerts/25"), + } + id = a.ID() + want = 88 + if !reflect.DeepEqual(id, want) { + t.Errorf("Alert.ID error returned %+v, want %+v", id, want) + } + + // Test: nil HTMLURL, valid URL + a = &Alert{ + URL: String("https://github.com/repos/o/r/code-scanning/alerts/25"), + } + id = a.ID() + want = 25 + if !reflect.DeepEqual(id, want) { + t.Errorf("Alert.ID error returned %+v, want %+v", id, want) + } + + // Test: Both HTMLURL and URL are nil + a = &Alert{} + id = a.ID() + want = 0 + if !reflect.DeepEqual(id, want) { + t.Errorf("Alert.ID error returned %+v, want %+v", id, want) + } + + // Test: ID can't be parsed as an int + a = &Alert{ + HTMLURL: String("https://github.com/o/r/security/code-scanning/bad88"), + URL: String("https://github.com/repos/o/r/code-scanning/alerts/25"), + } + id = a.ID() + want = 0 + if !reflect.DeepEqual(id, want) { + t.Errorf("Alert.ID error returned %+v, want %+v", id, want) + } +} + func TestActionsService_ListAlertsForRepo(t *testing.T) { client, mux, _, teardown := setup() defer teardown() From a5f2d78c6e33d9544d2276ab9eb049c0777e9809 Mon Sep 17 00:00:00 2001 From: Ryan Mast Date: Sat, 30 May 2020 19:41:29 -0700 Subject: [PATCH 5/7] Make suggested changes to Alert.ID() --- github/code-scanning.go | 33 ++++++++------------------------- github/code-scanning_test.go | 18 +++--------------- 2 files changed, 11 insertions(+), 40 deletions(-) diff --git a/github/code-scanning.go b/github/code-scanning.go index 4bf8ea399c..11541bb4a9 100644 --- a/github/code-scanning.go +++ b/github/code-scanning.go @@ -37,37 +37,20 @@ func (a *Alert) ID() int64 { return 0 } - // Set the url to check to "" and index of last forward slash to -1 - s := string("") - i := int(-1) + s := a.GetHTMLURL() - // Check if HTMLURL is valid - if a.HTMLURL != nil { - s = *a.HTMLURL - } - - // Check HTMLURL for an ID to parse first, since it was used in the example - if i = strings.LastIndex(s, "/"); i != -1 { + // Check for an ID to parse at the end of the url + if i := strings.LastIndex(s, "/"); i >= 0 { s = s[i+1:] - } else { - // HTMLURL had no forward slashes, try URL as a fallback - // Check if URL is valid - if a.URL != nil { - s = *a.URL - } - if i = strings.LastIndex(s, "/"); i != -1 { - s = s[i+1:] - } else { - // Both HTMLURL and URL were invalid and had no ID - return 0 - } } // Return the alert ID as a 64-bit integer. Unable to convert or out of range returns 0. - if id, err := strconv.ParseInt(s, 10, 64); err == nil { - return id + id, err := strconv.ParseInt(s, 10, 64) + if err != nil { + return 0 } - return 0 + + return id } // AlertListOptions specifies optional parameters to the CodeScanningService.ListAlerts diff --git a/github/code-scanning_test.go b/github/code-scanning_test.go index 9fc4206f5b..2e0c2282c4 100644 --- a/github/code-scanning_test.go +++ b/github/code-scanning_test.go @@ -16,9 +16,9 @@ import ( func TestActionsService_Alert_ID(t *testing.T) { // Test: nil Alert ID == 0 - a := (*Alert)(nil) + var a *Alert id := a.ID() - want := int64(0) + var want int64 if id != want { t.Errorf("Alert.ID error returned %+v, want %+v", id, want) } @@ -26,7 +26,6 @@ func TestActionsService_Alert_ID(t *testing.T) { // Test: Valid HTMLURL a = &Alert{ HTMLURL: String("https://github.com/o/r/security/code-scanning/88"), - URL: String("https://github.com/repos/o/r/code-scanning/alerts/25"), } id = a.ID() want = 88 @@ -34,17 +33,7 @@ func TestActionsService_Alert_ID(t *testing.T) { t.Errorf("Alert.ID error returned %+v, want %+v", id, want) } - // Test: nil HTMLURL, valid URL - a = &Alert{ - URL: String("https://github.com/repos/o/r/code-scanning/alerts/25"), - } - id = a.ID() - want = 25 - if !reflect.DeepEqual(id, want) { - t.Errorf("Alert.ID error returned %+v, want %+v", id, want) - } - - // Test: Both HTMLURL and URL are nil + // Test: HTMLURL is nil a = &Alert{} id = a.ID() want = 0 @@ -55,7 +44,6 @@ func TestActionsService_Alert_ID(t *testing.T) { // Test: ID can't be parsed as an int a = &Alert{ HTMLURL: String("https://github.com/o/r/security/code-scanning/bad88"), - URL: String("https://github.com/repos/o/r/code-scanning/alerts/25"), } id = a.ID() want = 0 From cb1bbde39f5e6a3c619ca476407640d745cb33e6 Mon Sep 17 00:00:00 2001 From: Ryan Mast Date: Tue, 2 Jun 2020 20:23:43 -0700 Subject: [PATCH 6/7] Switch ClosedBy field to type User based on GH support response --- github/code-scanning.go | 2 +- github/github-accessors.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/github/code-scanning.go b/github/code-scanning.go index 11541bb4a9..9cbd0b0dcd 100644 --- a/github/code-scanning.go +++ b/github/code-scanning.go @@ -25,7 +25,7 @@ type Alert struct { Tool *string `json:"tool,omitempty"` CreatedAt *Timestamp `json:"created_at,omitempty"` Open *bool `json:"open,omitempty"` - ClosedBy *string `json:"closed_by,omitempty"` + ClosedBy *User `json:"closed_by,omitempty"` ClosedAt *Timestamp `json:"closed_at,omitempty"` URL *string `json:"url,omitempty"` HTMLURL *string `json:"html_url,omitempty"` diff --git a/github/github-accessors.go b/github/github-accessors.go index 2fa325ddbc..5e2fbb6826 100644 --- a/github/github-accessors.go +++ b/github/github-accessors.go @@ -116,12 +116,12 @@ func (a *Alert) GetClosedAt() Timestamp { return *a.ClosedAt } -// GetClosedBy returns the ClosedBy field if it's non-nil, zero value otherwise. -func (a *Alert) GetClosedBy() string { - if a == nil || a.ClosedBy == nil { - return "" +// GetClosedBy returns the ClosedBy field. +func (a *Alert) GetClosedBy() *User { + if a == nil { + return nil } - return *a.ClosedBy + return a.ClosedBy } // GetCreatedAt returns the CreatedAt field if it's non-nil, zero value otherwise. From 0b014cc57db3ec52548cdc2c525f2e657d91f96a Mon Sep 17 00:00:00 2001 From: Glenn Lewis <6598971+gmlewis@users.noreply.github.com> Date: Tue, 16 Jun 2020 21:15:36 -0400 Subject: [PATCH 7/7] Fix comment for auto-generated godocs --- github/code-scanning.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github/code-scanning.go b/github/code-scanning.go index 9cbd0b0dcd..dc57a89025 100644 --- a/github/code-scanning.go +++ b/github/code-scanning.go @@ -31,7 +31,7 @@ type Alert struct { HTMLURL *string `json:"html_url,omitempty"` } -// ID() returns the ID associated with an alert. It is the number at the end of the security alert's URL. +// ID returns the ID associated with an alert. It is the number at the end of the security alert's URL. func (a *Alert) ID() int64 { if a == nil { return 0