Maintenance: Fix code quality issues in parameters package

[dreamorosi]

Summary

The packages/parameters package contains 9 code quality issues identified by SonarQube that affect parameter retrieval functionality and test reliability:

1. Readonly Member Issues (4 MAJOR issues): Class members that are never reassigned after initialization but are not marked as readonly

   • packages/parameters/src/appconfig/AppConfigProvider.ts (2 members: lines 192, 193)
   • packages/parameters/tests/unit/BaseProvider.test.ts (1 member: line 27)
   • packages/parameters/tests/helpers/tinyLogger.ts (1 member: line 16)

2. Import Issues (2 MINOR issues): Duplicate imports that can be consolidated

   • packages/parameters/src/dynamodb/DynamoDBProvider.ts (lines 5, 11)

3. Union Type Issues (1 MINOR issue): Type union optimization needed

   • packages/parameters/src/base/BaseProvider.ts (line 95)

4. Test Code Issues (2 MINOR issues): Unnecessary type assertions in tests

   • packages/parameters/tests/unit/SSMProvider.test.ts (lines 1015, 1235)

These issues affect core parameter providers, base functionality, and test reliability across the Parameters package.

Why is this needed?

1. Code Quality: Improves code clarity by explicitly marking immutable members as readonly
2. SonarQube Compliance: Resolves 9 code quality issues (typescript:S2933, typescript:S3863, typescript:S6571, typescript:S4325)
3. Type Safety: Prevents accidental reassignment of critical configuration members
4. Import Optimization: Reduces bundle size and improves build performance
5. Developer Intent: Makes the immutable nature of these members explicit to future maintainers
6. Best Practices: Follows TypeScript best practices for class member declarations and imports

The Parameters package is a foundational utility for retrieving configuration from AWS services like SSM, Secrets Manager, and AppConfig, so maintaining high code quality standards here is particularly important for reliability.

Solution

Important

The following changes are included as reference to help you understand the refactoring. Before implementing, please make sure to check the codebase and ensure that they make sense and they are exhaustive.

1. Fix AppConfigProvider.ts readonly members (2 issues)

Current code:

class AppConfigProvider extends BaseProvider {
  // ...
  application: string;
  environment: string;
  // ...
}

Improved code:

class AppConfigProvider extends BaseProvider {
  // ...
  readonly application: string;
  readonly environment: string;
  // ...
}

2. Fix BaseProvider.test.ts readonly member (1 issue)

Current code:

class TestProvider extends BaseProvider {
  // ...
  #name: string;
  // ...
}

Improved code:

class TestProvider extends BaseProvider {
  // ...
  readonly #name: string;
  // ...
}

3. Fix tinyLogger.ts readonly member (1 issue)

Current code:

class TinyLogger {
  // ...
  console: Console;
  // ...
}

Improved code:

class TinyLogger {
  // ...
  readonly console: Console;
  // ...
}

4. Fix DynamoDBProvider.ts duplicate imports (2 issues)

Current code:

import { DynamoDBClient } from '@aws-sdk/client-dynamodb';
// ... other imports
import { GetItemCommand, QueryCommand } from '@aws-sdk/client-dynamodb';

Improved code:

import { 
  DynamoDBClient,
  GetItemCommand, 
  QueryCommand 
} from '@aws-sdk/client-dynamodb';
// ... other imports

5. Fix BaseProvider.ts union type (1 issue)

Current code:

// Union type where 'unknown' overrides other types
public async get(
  name: string,
  options?: GetOptionsInterface
): Promise<unknown | undefined> {

Improved code:

// Remove 'undefined' from union
public async get(
  name: string,
  options?: GetOptionsInterface
): Promise<unknown> {

6. Fix SSMProvider.test.ts unnecessary assertions (2 issues)

Current code:

// Unnecessary type assertions that don't change the type
expect(result as SomeType).toBeDefined();
expect(value as string).toEqual('expected');

Improved code:

// Remove unnecessary type assertions
expect(result).toBeDefined();
expect(value).toEqual('expected');

These changes:

• Make the immutable nature of class members explicit
• Prevent accidental reassignment in future code changes
• Optimize import statements for better build performance
• Fix union type issues for better type safety
• Remove unnecessary type assertions in tests
• Follow TypeScript best practices for class design and imports

Implementation Details

1. Files to modify:

   • packages/parameters/src/appconfig/AppConfigProvider.ts (lines 192, 193)
   • packages/parameters/tests/unit/BaseProvider.test.ts (line 27)
   • packages/parameters/tests/helpers/tinyLogger.ts (line 16)
   • packages/parameters/src/dynamodb/DynamoDBProvider.ts (lines 5, 11)
   • packages/parameters/src/base/BaseProvider.ts (line 95)
   • packages/parameters/tests/unit/SSMProvider.test.ts (lines 1015, 1235)

2. Testing:

   • Ensure all existing tests continue to pass
   • Run type checking to verify no TypeScript errors are introduced
   • Verify that the readonly modifier doesn't break any existing functionality
   • Confirm import consolidation doesn't affect functionality

3. Validation:

   • Run npm run test:unit -w packages/parameters to ensure no regressions
   • Run npm run lint -w packages/parameters to verify code style compliance
   • Run npm run build -ws to verify TypeScript compilation
   • Run SonarQube analysis to confirm issues are resolved

Additional Context

This issue was identified as part of a SonarQube code quality review. The Parameters package is a foundational utility for retrieving configuration from AWS services like SSM Parameter Store, AWS Secrets Manager, and AWS AppConfig, making code quality improvements here particularly valuable for reliability and maintainability.

These changes are primarily declarative improvements that should not change any functionality or behavior - they simply make the immutable nature of class members explicit and optimize imports.

SonarQube Issue Keys:

• AZItlgSpcEUs2753LD6M - AppConfigProvider.ts application
• AZItlgSpcEUs2753LD6N - AppConfigProvider.ts environment
• AZItlgUBcEUs2753LD6O - BaseProvider.test.ts #name
• AZItlgVTcEUs2753LD6P - tinyLogger.ts console
• AY1fFCoNqFe1DseJcRV6 - DynamoDBProvider.ts duplicate import (line 5)
• AY1fFCoNqFe1DseJcRV7 - DynamoDBProvider.ts duplicate import (line 11)
• AYlpRzOxOvI06ztqLT9P - BaseProvider.ts union type
• AY99NK8Sr_nD7WwQSUhP - SSMProvider.test.ts unnecessary assertion (line 1015)
• AY99NK8Sr_nD7WwQSUhQ - SSMProvider.test.ts unnecessary assertion (line 1235)

Acknowledgment

• This request meets Powertools for AWS Lambda (TypeScript) Tenets
• Should this be considered in other Powertools for AWS Lambda languages? i.e. Python, Java, and .NET

Future readers

Please react with 👍 and your use case to help us understand customer demand.

[shrivarshapoojari]

Hi,
I would like to work on this issue.
Could you please assign this to me?
Thanks

[leandrodamascena]

Hi, I would like to work on this issue. Could you please assign this to me? Thanks

Hi @shrivarshapoojari! Thanks for offering to help me working in this issue! I'm assigning this to you, and please let us know if you have any questions we can help with.

[shrivarshapoojari]

Hi,could you please elaborate on resolving "Test Code Issues (2 MINOR issues): Unnecessary type assertions in tests".
I think attached SonarQube issue keys are pointing to different files instead of pointing to packages/parameters/tests/unit/SSMProvider.test.ts

And also could you please share any doc on how to run SonarQube analysis for this project ..

Thanks

[leandrodamascena]

Hi,could you please elaborate on resolving "Test Code Issues (2 MINOR issues): Unnecessary type assertions in tests". I think attached SonarQube issue keys are pointing to different files instead of pointing to packages/parameters/tests/unit/SSMProvider.test.ts

Hey @dreamorosi can you pls clarify this?

And also could you please share any doc on how to run SonarQube analysis for this project ..

You don't need to worry about this, when you open a PR and when you change files in this PR, we automatically run a Sonar analysis that will report the current status of that PR.

[sdangol]

Hi,could you please elaborate on resolving "Test Code Issues (2 MINOR issues): Unnecessary type assertions in tests". I think attached SonarQube issue keys are pointing to different files instead of pointing to packages/parameters/tests/unit/SSMProvider.test.ts

@shrivarshapoojari These should be the ones pointing to the correct files: https://sonarcloud.io/project/issues?fileUuids=AYlpRy3FOvI06ztqLT3O&issueStatuses=OPEN%2CCONFIRMED&id=aws-powertools_powertools-lambda-typescript

[shrivarshapoojari]

@shrivarshapoojari These should be the ones pointing to the correct files: https://sonarcloud.io/project/issues?fileUuids=AYlpRy3FOvI06ztqLT3O&issueStatuses=OPEN%2CCONFIRMED&id=aws-powertools_powertools-lambda-typescript

Hi,
I think ,The as GetParametersCommandOutput assertions should be kept because:

1. TypeScript Compilation: Removing them causes compilation errors since the methods expect complete GetParametersCommandOutput objects.

2. Test Simplicity: Current tests use minimal partial objects (only InvalidParameters or Parameters arrays) without AWS SDK metadata.

3. If assertions are removed: All 6 test cases would need to provide complete objects with $metadata:

   • Lines 1029, 1043, 1058: Add $metadata to result objects
   • Lines 1249, 1267, 1294: Add $metadata to response objects

This would make tests verbose..

Could you please share your thoughts..

Thanks.