update linter

JohnN193 · May 23, 2024 · 1b0153d · 1b0153d
1 parent 03a9544
commit 1b0153d
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 18 deletions.
diff --git a/etc/golangci.yaml b/etc/golangci.yaml
@@ -12,12 +12,12 @@ linters:
     - contextcheck
     - cyclop
     - deadcode
+    - depguard
     - exhaustivestruct
     - exhaustruct
     - forcetypeassert
     - funlen
     - gocognit
-    - gocritic
     - godox
     - goerr113
     - gochecknoglobals
@@ -36,6 +36,7 @@ linters:
     - maligned
     - makezero
     - musttag
+    - nakedret
     - nestif
     - nlreturn
     - nosnakecase
@@ -60,8 +61,12 @@ linters-settings:
       - standard
       - default
       - prefix(github.com/viamrobotics/viam-cartographer)
+  gocritic:
+    enabled-tags:
+    disabled-checks:
+      - dupSubExpr
   gofumpt:
-    lang-version: "1.19"
+    lang-version: "1.21"
     extra-rules: true
   gosec:
     excludes:
@@ -71,6 +76,39 @@ linters-settings:
     disable:
       - fieldalignment
       - shadow
+      - composites
+  revive:
+    # Unfortunately configuring a single rules disables all other rules, even
+    # if we set `enable-all: true`
+    #
+    # To get around this, we include default rules:
+    # https://github.com/mgechev/revive/blob/master/defaults.toml
+    rules:
+      - name: blank-imports
+      - name: context-as-argument
+        disabled: false
+        arguments:
+          - allowTypesBefore: "testing.TB,*testing.T,*testing.B,*testing.F"
+      - name: context-keys-type
+      - name: dot-imports
+      - name: empty-block
+      - name: error-naming
+      - name: error-return
+      - name: error-strings
+      - name: errorf
+      - name: exported
+      - name: increment-decrement
+      - name: indent-error-flow
+      - name: package-comments
+      - name: range
+      - name: receiver-naming
+      - name: redefines-builtin-id
+      - name: superfluous-else
+      - name: time-naming
+      - name: unexported-return
+      - name: unreachable-code
+      - name: var-declaration
+      - name: var-naming
   lll:
     line-length: 140
 issues:

diff --git a/sensorprocess/testhelper.go b/sensorprocess/testhelper.go
@@ -111,14 +111,15 @@ func validAddLidarReadingInOnlineTestHelper(
 		test.That(t, call.timeout, test.ShouldEqual, config.Timeout)
 	}
 
-	if testLidar == s.GoodLidar {
+	switch {
+	case testLidar == s.GoodLidar:
 		test.That(t, calls[0].currentReading.ReadingTime.Before(calls[1].currentReading.ReadingTime), test.ShouldBeTrue)
 		test.That(t, calls[1].currentReading.ReadingTime.Before(calls[2].currentReading.ReadingTime), test.ShouldBeTrue)
-	} else if testLidar == s.ReplayLidar {
+	case testLidar == s.ReplayLidar:
 		readingTime, err := time.Parse(time.RFC3339Nano, s.TestTimestamp)
 		test.That(t, err, test.ShouldBeNil)
 		test.That(t, calls[0].currentReading.ReadingTime.Equal(readingTime), test.ShouldBeTrue)
-	} else {
+	default:
 		t.Errorf("no timestamp tests provided for %v", string(testLidar))
 	}
 }

diff --git a/sensors/movementsensor.go b/sensors/movementsensor.go
@@ -324,11 +324,12 @@ func NewMovementSensor(
 }
 
 func averageReadingTimes(a, b time.Time) time.Time {
-	if b.Equal(a) {
+	switch {
+	case b.Equal(a):
 		return a
-	} else if b.After(a) {
+	case b.After(a):
 		return a.Add(b.Sub(a) / 2)
-	} else {
+	default:
 		return b.Add(a.Sub(b) / 2)
 	}
 }
diff --git a/sensors/test_deps.go b/sensors/test_deps.go
@@ -138,20 +138,20 @@ var (
 		IMUWithErroringFunctions:             getIMUWithErroringFunctions,
 		ReplayIMU:                            func() *inject.MovementSensor { return getReplayIMU(TestTimestamp) },
 		InvalidReplayIMU:                     func() *inject.MovementSensor { return getReplayIMU(BadTime) },
-		FinishedReplayIMU:                    func() *inject.MovementSensor { return getFinishedReplayIMU() },
+		FinishedReplayIMU:                    getFinishedReplayIMU,
 		GoodOdometer:                         getGoodOdometer,
 		OdometerWithErroringFunctions:        getOdometerWithErroringFunctions,
 		ReplayOdometer:                       func() *inject.MovementSensor { return getReplayOdometer(TestTimestamp) },
 		InvalidReplayOdometer:                func() *inject.MovementSensor { return getReplayOdometer(BadTime) },
-		FinishedReplayOdometer:               func() *inject.MovementSensor { return getFinishedReplayOdometer() },
+		FinishedReplayOdometer:               getFinishedReplayOdometer,
 		MovementSensorNotIMUNotOdometer:      getMovementSensorNotIMUAndNotOdometer,
 		GoodMovementSensorBothIMUAndOdometer: getGoodMovementSensorBothIMUAndOdometer,
 		MovementSensorBothIMUAndOdometerWithErroringFunctions: getMovementSensorBothIMUAndOdometerWithErroringFunctions,
 		MovementSensorWithErroringPropertiesFunc:              getMovementSensorWithErroringPropertiesFunc,
 		MovementSensorWithInvalidProperties:                   getMovementSensorWithInvalidProperties,
 		ReplayMovementSensorBothIMUAndOdometer:                func() *inject.MovementSensor { return getReplayMovementSensor(TestTimestamp) },
 		InvalidReplayMovementSensorBothIMUAndOdometer:         func() *inject.MovementSensor { return getReplayMovementSensor(BadTime) },
-		FinishedReplayMovementSensor:                          func() *inject.MovementSensor { return getFinishedReplayMovementSensor() },
+		FinishedReplayMovementSensor:                          getFinishedReplayMovementSensor,
 	}
 )
 

diff --git a/viam-cartographer/src/carto_facade/carto_facade.cc b/viam-cartographer/src/carto_facade/carto_facade.cc
@@ -178,7 +178,7 @@ CartoFacade::CartoFacade(viam_carto_lib *pVCL, const viam_carto_config c,
     path_to_internal_state_file = config.existing_map;
 };
 
-CartoFacade::~CartoFacade() {  }
+CartoFacade::~CartoFacade() {}
 
 void CartoFacade::IOInit() {
     if (state != CartoFacadeState::INITIALIZED) {
@@ -644,12 +644,12 @@ void CartoFacade::AddLidarReading(const viam_carto_lidar_reading *sr) {
     bstring camera_sensor = to_bstring(config.camera);
     bool known_sensor = biseq(camera_sensor, sr->lidar);
     bdestroy(camera_sensor);
-     if (!known_sensor) {
+    if (!known_sensor) {
         VLOG(1) << "expected sensor: " << to_std_string(sr->lidar) << " to be "
                 << config.camera;
         throw VIAM_CARTO_UNKNOWN_SENSOR_NAME;
     }
-    
+
     std::string lidar_reading = to_std_string(sr->lidar_reading);
     if (lidar_reading.length() == 0) {
         throw VIAM_CARTO_LIDAR_READING_EMPTY;

diff --git a/viam_cartographer.go b/viam_cartographer.go
@@ -695,13 +695,14 @@ func (cartoSvc *CartographerService) Properties(ctx context.Context) (slam.Prope
 		props.SensorInfo = append(props.SensorInfo, slam.SensorInfo{Name: cartoSvc.movementSensor.Name(), Type: slam.SensorTypeMovementSensor})
 	}
 
-	if cartoSvc.enableMapping && cartoSvc.existingMap == "" {
+	switch {
+	case cartoSvc.enableMapping && cartoSvc.existingMap == "":
 		props.MappingMode = slam.MappingModeNewMap
-	} else if cartoSvc.enableMapping && cartoSvc.existingMap != "" {
+	case cartoSvc.enableMapping && cartoSvc.existingMap != "":
 		props.MappingMode = slam.MappingModeUpdateExistingMap
-	} else if !cartoSvc.enableMapping && cartoSvc.existingMap != "" {
+	case !cartoSvc.enableMapping && cartoSvc.existingMap != "":
 		props.MappingMode = slam.MappingModeLocalizationOnly
-	} else {
+	default:
 		return slam.Properties{}, errors.New("invalid mode: localizing requires an existing map")
 	}