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

fix(linter): Upgrade and add some linters #1059

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,5 @@ jobs:
- name: Run linter
uses: golangci/golangci-lint-action@v3
with:
version: v1.56.2
version: v1.59.1

119 changes: 103 additions & 16 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
linters-settings:
govet:
check-shadowing: true
settings:
printf:
funcs:
Expand All @@ -10,68 +9,156 @@ linters-settings:
- (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf
gocyclo:
min-complexity: 15
maligned:
suggest-new: true
unparam:
check-exported: false
gomnd:
settings:
mnd:
checks: argument,case,condition,return
ignored-numbers: 1,2,3,5,10,60,64,100,600,0755,0644,0666
revive:
enable-all-rules: true
rules:
- name: unhandled-error
disabled: true
- name: unexported-return
disabled: true
- name: line-length-limit
disabled: true
- name: add-constant
disabled: true
- name: cognitive-complexity
disabled: true
- name: cyclomatic
disabled: true
- name: flag-parameter
disabled: true
- name: unused-receiver
disabled: true
- name: unchecked-type-assertion
disabled: true
- name: get-return
disabled: true
- name: function-length
disabled: true
- name: import-shadowing
disabled: true
- name: bare-return
disabled: true
- name: import-alias-naming
disabled: true
- name: function-result-limit
disabled: true
- name: confusing-naming
disabled: true
- name: receiver-naming
disabled: true
- name: var-naming
disabled: true
- name: confusing-results
disabled: true
- name: max-public-structs
disabled: true
- name: deep-exit
disabled: true
- name: modifies-value-receiver
disabled: true
- name: exported
disabled: true
- name: unnecessary-stmt
disabled: true

mnd:
checks:
- argument
- case
- condition
- return
ignored-numbers:
- '1'
- '2'
- '3'
- '5'
- '10'
- '30'
- '60'
- '64'
- '100'
- '600'
- '0755'
- '0644'
- '0666'
- '0o666'
- '0o644'
- '0o755'

linters:
# inverted configuration with `enable-all` and `disable` is not scalable during updates of golangci-lint
disable-all: true
enable:
- asasalint
- asciicheck
- bidichk
- bodyclose
- contextcheck
- decorder
- dogsled
- durationcheck
- errcheck
- errorlint
- exportloopref
- goconst
- fatcontext
- ginkgolinter
- gocheckcompilerdirectives
- gochecksumtype
- goconst
- gocyclo
- goconst
- godot
- gofmt
- gofumpt
- goheader
- goimports
- gomnd
- gosimple
- govet
- grouper
- ineffassign
- loggercheck
- makezero
- misspell
- mnd
- nilerr
- noctx
- nosprintfhostport
- prealloc
- predeclared
- promlinter
- protogetter
- reassign
- revive
- rowserrcheck
- sloglint
- spancheck
- sqlclosecheck
- staticcheck
- testifylint
- tparallel
- typecheck
- unconvert
- unparam
- unused
- usestdlibvars
- wastedassign
- whitespace

issues:
exclude-dirs:
- vendor
exclude-files:
- notifier/registrator.go
exclude-rules:
- path: _test\.go
linters:
- gomnd
- contextcheck
- mnd
- err113
- errcheck
- goconst
- revive

run:
timeout: 5m
skip-files:
- notifier/registrator.go
skip-dirs:
- vendor
4 changes: 2 additions & 2 deletions api/authorization.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ func (auth *Authorization) IsAdmin(login string) bool {
type Role string

var (
RoleUndefined Role = ""
RoleUndefined Role
RoleUser Role = "user"
RoleAdmin Role = "admin"
)

// Returns the role of the given user.
// GetRole Returns the role of the given user.
func (auth *Authorization) GetRole(login string) Role {
if !auth.IsEnabled() {
return RoleUndefined
Expand Down
4 changes: 2 additions & 2 deletions api/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ type WebConfig struct {
// MetricSourceCluster contains data about supported metric source cluster.
type MetricSourceCluster struct {
TriggerSource moira.TriggerSource `json:"trigger_source" example:"graphite_remote"`
ClusterId moira.ClusterId `json:"cluster_id" example:"default"`
ClusterID moira.ClusterID `json:"cluster_id" example:"default"`
ClusterName string `json:"cluster_name" example:"Graphite Remote Prod"`
}

func (WebConfig) Render(w http.ResponseWriter, r *http.Request) error {
func (WebConfig) Render(http.ResponseWriter, *http.Request) error {
return nil
}
5 changes: 3 additions & 2 deletions api/controller/contact.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ func GetAllContacts(database moira.Database) (*dto.ContactList, *api.ErrorRespon
return &contactsList, nil
}

// GetContactById gets notification contact by its id string.
func GetContactById(database moira.Database, contactID string) (*dto.Contact, *api.ErrorResponse) {
// GetContactByID gets notification contact by its id string.
func GetContactByID(database moira.Database, contactID string) (*dto.Contact, *api.ErrorResponse) {
contact, err := database.GetContact(contactID)
if err != nil {
return nil, api.ErrorInternalServer(err)
Expand Down Expand Up @@ -160,6 +160,7 @@ func RemoveContact(database moira.Database, contactID string, userLogin string,
if contact == contactID {
subscription.Contacts = append(subscription.Contacts[:i], subscription.Contacts[i+1:]...)
subscriptionsWithDeletingContact = append(subscriptionsWithDeletingContact, subscription)

break
}
}
Expand Down
5 changes: 3 additions & 2 deletions api/controller/contact_events.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ import (
"github.com/moira-alert/moira/api/dto"
)

func GetContactEventsByIdWithLimit(database moira.Database, contactID string, from int64, to int64) (*dto.ContactEventItemList, *api.ErrorResponse) {
events, err := database.GetNotificationsByContactIdWithLimit(contactID, from, to)
// GetContactEventsByIDWithLimit func returns events by contact id with limit.
func GetContactEventsByIDWithLimit(database moira.Database, contactID string, from int64, to int64) (*dto.ContactEventItemList, *api.ErrorResponse) {
events, err := database.GetNotificationsByContactIDWithLimit(contactID, from, to)
if err != nil {
return nil, api.ErrorInternalServer(fmt.Errorf("GetContactEventsByIdWithLimit: can't get notifications for contact with id %v", contactID))
}
Expand Down
12 changes: 6 additions & 6 deletions api/controller/contact_events_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,19 +79,19 @@ func TestGetContactEventsByIdWithLimit(t *testing.T) {

Convey("Ensure that request with default parameters would return both event items (no url params specified)", t, func() {
dataBase.EXPECT().GetContact(contact.ID).Return(contactExpect, nil).AnyTimes()
dataBase.EXPECT().GetNotificationsByContactIdWithLimit(contact.ID, defaultFromParameter, defaultToParameter).Return(items, nil)
dataBase.EXPECT().GetNotificationsByContactIDWithLimit(contact.ID, defaultFromParameter, defaultToParameter).Return(items, nil)

actualEvents, err := GetContactEventsByIdWithLimit(dataBase, contact.ID, defaultFromParameter, defaultToParameter)
actualEvents, err := GetContactEventsByIDWithLimit(dataBase, contact.ID, defaultFromParameter, defaultToParameter)

So(err, ShouldBeNil)
So(actualEvents, ShouldResemble, &itemsExpected)
})

Convey("Ensure that request with only 'from' parameter given and 'to' default will return only one (newest) event", t, func() {
dataBase.EXPECT().GetContact(contact.ID).Return(contactExpect, nil).AnyTimes()
dataBase.EXPECT().GetNotificationsByContactIdWithLimit(contact.ID, defaultFromParameter-20, defaultToParameter).Return(items[:1], nil)
dataBase.EXPECT().GetNotificationsByContactIDWithLimit(contact.ID, defaultFromParameter-20, defaultToParameter).Return(items[:1], nil)

actualEvents, err := GetContactEventsByIdWithLimit(dataBase, contact.ID, defaultFromParameter-20, defaultToParameter)
actualEvents, err := GetContactEventsByIDWithLimit(dataBase, contact.ID, defaultFromParameter-20, defaultToParameter)
So(err, ShouldBeNil)
So(actualEvents, ShouldResemble, &dto.ContactEventItemList{
List: []dto.ContactEventItem{
Expand All @@ -102,9 +102,9 @@ func TestGetContactEventsByIdWithLimit(t *testing.T) {

Convey("Ensure that request with only 'to' parameter given and 'from' default will return only one (oldest) event", t, func() {
dataBase.EXPECT().GetContact(contact.ID).Return(contactExpect, nil).AnyTimes()
dataBase.EXPECT().GetNotificationsByContactIdWithLimit(contact.ID, defaultFromParameter, defaultToParameter-30).Return(items[1:], nil)
dataBase.EXPECT().GetNotificationsByContactIDWithLimit(contact.ID, defaultFromParameter, defaultToParameter-30).Return(items[1:], nil)

actualEvents, err := GetContactEventsByIdWithLimit(dataBase, contact.ID, defaultFromParameter, defaultToParameter-30)
actualEvents, err := GetContactEventsByIDWithLimit(dataBase, contact.ID, defaultFromParameter, defaultToParameter-30)
So(err, ShouldBeNil)
So(actualEvents, ShouldResemble, &dto.ContactEventItemList{
List: []dto.ContactEventItem{
Expand Down
10 changes: 5 additions & 5 deletions api/controller/contact_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func TestGetContactById(t *testing.T) {
}

dataBase.EXPECT().GetContact(contact.ID).Return(contact, nil)
actual, err := GetContactById(dataBase, contact.ID)
actual, err := GetContactByID(dataBase, contact.ID)
So(err, ShouldBeNil)
So(actual,
ShouldResemble,
Expand All @@ -88,9 +88,9 @@ func TestGetContactById(t *testing.T) {
})

Convey("Get contact with invalid or unexisting guid id should be empty json", t, func() {
const invalidId = "invalidID"
dataBase.EXPECT().GetContact(invalidId).Return(moira.ContactData{}, nil)
actual, err := GetContactById(dataBase, invalidId)
const invalidID = "invalidID"
dataBase.EXPECT().GetContact(invalidID).Return(moira.ContactData{}, nil)
actual, err := GetContactByID(dataBase, invalidID)
So(err, ShouldBeNil)
So(actual, ShouldResemble, &dto.Contact{})
})
Expand All @@ -101,7 +101,7 @@ func TestGetContactById(t *testing.T) {
dbError := fmt.Errorf("some db internal error here")

dataBase.EXPECT().GetContact(contactID).Return(emptyContact, dbError)
contact, err := GetContactById(dataBase, contactID)
contact, err := GetContactByID(dataBase, contactID)
So(err, ShouldResemble, api.ErrorInternalServer(dbError))
So(contact, ShouldBeNil)
})
Expand Down
13 changes: 6 additions & 7 deletions api/controller/team.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func GetTeam(dataBase moira.Database, teamID string) (dto.TeamModel, *api.ErrorR
return teamModel, nil
}

// GetUserTeams is a controller function that returns a teams in which user is a member bu user ID.
// GetUserTeams is a controller function that returns a teams in which user is a member by user ID.
func GetUserTeams(dataBase moira.Database, userID string) (dto.UserTeams, *api.ErrorResponse) {
teams, err := dataBase.GetUserTeams(userID)

Expand Down Expand Up @@ -406,7 +406,6 @@ func CheckUserPermissionsForTeam(
if auth.IsAdmin(userID) {
return nil
}

_, err := dataBase.GetTeam(teamID)
if err != nil {
if errors.Is(err, database.ErrNil) {
Expand All @@ -426,19 +425,19 @@ func CheckUserPermissionsForTeam(
}

// GetTeamSettings gets team contacts and subscriptions.
func GetTeamSettings(database moira.Database, teamID string) (dto.TeamSettings, *api.ErrorResponse) {
func GetTeamSettings(dataBase moira.Database, teamID string) (dto.TeamSettings, *api.ErrorResponse) {
Copy link
Member

Choose a reason for hiding this comment

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

А почему так?

Copy link
Member Author

Choose a reason for hiding this comment

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

есть пакет database - одинаковый нейминг)

Copy link
Member

Choose a reason for hiding this comment

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

Может назовём переменную db или это ещё хуже?

teamSettings := dto.TeamSettings{
TeamID: teamID,
Contacts: make([]moira.ContactData, 0),
Subscriptions: make([]moira.SubscriptionData, 0),
}

subscriptionIDs, err := database.GetTeamSubscriptionIDs(teamID)
subscriptionIDs, err := dataBase.GetTeamSubscriptionIDs(teamID)
if err != nil {
return dto.TeamSettings{}, api.ErrorInternalServer(err)
}

subscriptions, err := database.GetSubscriptions(subscriptionIDs)
subscriptions, err := dataBase.GetSubscriptions(subscriptionIDs)
if err != nil {
return dto.TeamSettings{}, api.ErrorInternalServer(err)
}
Expand All @@ -447,12 +446,12 @@ func GetTeamSettings(database moira.Database, teamID string) (dto.TeamSettings,
teamSettings.Subscriptions = append(teamSettings.Subscriptions, *subscription)
}
}
contactIDs, err := database.GetTeamContactIDs(teamID)
contactIDs, err := dataBase.GetTeamContactIDs(teamID)
if err != nil {
return dto.TeamSettings{}, api.ErrorInternalServer(err)
}

contacts, err := database.GetContacts(contactIDs)
contacts, err := dataBase.GetContacts(contactIDs)
if err != nil {
return dto.TeamSettings{}, api.ErrorInternalServer(err)
}
Expand Down
6 changes: 3 additions & 3 deletions api/controller/trigger_metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ func TestGetTriggerMetrics(t *testing.T) {
ID: triggerID,
Targets: []string{pattern},
TriggerSource: moira.PrometheusRemote,
ClusterId: moira.DefaultCluster,
ClusterID: moira.DefaultCluster,
}
dataBase.EXPECT().GetTrigger(triggerID).Return(trigger, nil)
triggerMetrics, err := GetTriggerMetrics(dataBase, sourceProvider, from, until, triggerID)
Expand All @@ -295,7 +295,7 @@ func TestGetTriggerMetrics(t *testing.T) {
ID: triggerID,
Targets: []string{pattern},
TriggerSource: moira.GraphiteLocal,
ClusterId: moira.DefaultCluster,
ClusterID: moira.DefaultCluster,
}, nil)
localSource.EXPECT().Fetch(pattern, from, until, false).Return(fetchResult, nil)
fetchResult.EXPECT().GetMetricsData().Return([]metricSource.MetricData{*metricSource.MakeMetricData(metric, []float64{0, 1, 2, 3, 4}, retention, from)})
Expand Down Expand Up @@ -328,7 +328,7 @@ func TestGetTriggerMetrics(t *testing.T) {
ID: triggerID,
Targets: []string{pattern},
TriggerSource: moira.GraphiteRemote,
ClusterId: moira.DefaultCluster,
ClusterID: moira.DefaultCluster,
}, nil)
remoteSource.EXPECT().Fetch(pattern, from, until, false).Return(nil, expectedError)

Expand Down
Loading
Loading