Apply SimplifyInterpolationRule to all expressions

[jeskew]

Fixes #5963

Update the SimplifyInterpolationRule linter rule to apply to all expressions that resolve to strings, not just identifiers of string variables and parameters.

Contributing a Pull Request

If you haven't already, read the full contribution guide. The guide may have changed since the last time you read it, so please double-check. Once you are done and ready to submit your PR, run through the relevant checklist below.

Contributing a feature

• I have opened a new issue for the proposal, or commented on an existing one, and ensured that the Bicep maintainers are good with the design of the feature being implemented
• I have included "Fixes #{issue_number}" in the PR description, so GitHub can link to the issue and close it when the PR is merged
• I have appropriate test coverage of my new feature

[ghost]

All CLA requirements met.

[majastrz]

@jeskew There are some baseline test failures in. In the contribution guide, there are some instructions on how to force the tests to regenerate the baseline assets. Then, you just manually review the git diffs to ensure that the changes are legitimate.

[StephenWeatherford]

@jeskew Thanks! I should be able to take a look today or tomorrow. Would love to have this fixed.

[majastrz]

Thanks for fixing!

[StephenWeatherford]

Re-running suites (timeout issue?)

[StephenWeatherford]

@majastrz My original code had this comment indicating why it wasn't done:

// We only want to trigger if the var or param is of type string (because interpolation
// using non-string types can be a perfectly valid way to convert to string, e.g. '${intVar}')

It sounds like the rule should only trigger if the expression type is string? Does that sound right to you?

[StephenWeatherford]

We need to resolve the comment I made to Marcin before merging

[jeskew]

@StephenWeatherford The SemanticModel.GetTypeInfo method is able to determine the resolved type of complex expressions, so the use case you mention is already accounted for. I'll add some test cases to make sure expressions like '${max(1, 2)}' or '${concat(<array>, <array>)}' are left as is.

[StephenWeatherford]

Ah, I missed that there was aleady a IsStrictlyAssignableToString(type)) check. Thanks!

[StephenWeatherford]

Trying build again - I've had some issues with it recently.

[jeskew]

@StephenWeatherford The failing tests are reporting errors about being unable to locate credentials for live tests. It looks like these tests require access to a couple repository secrets, so I believe those failures are expected given GitHub's policy of not sharing secrets with pull requests originating from forks.

[majastrz]

Live tests aren't marked as required. We'll be updating the CI configuration to exclude them in fork PRs to avoid confusion in the future.