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

Add trace log message when failing to find Env Var #13777

Merged
merged 7 commits into from
Jun 12, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
30 changes: 29 additions & 1 deletion src/Bicep.Core.IntegrationTests/ParameterFileTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,37 @@ public void Parameters_file_cannot_reference_non_existing_env_variable()

result.ExcludingLinterDiagnostics().Should().HaveDiagnostics(new[]{
("BCP338", DiagnosticLevel.Error,
"Failed to evaluate parameter \"stringEnvVariable\": Environment variable does not exist, and no default value set")});
SimonWahlin marked this conversation as resolved.
Show resolved Hide resolved
"Failed to evaluate parameter \"fromEnv\": Environment variable \"stringEnvVariable\" does not exist, and no default value set.")});
}

[TestMethod]
public void Parameters_file_cannot_reference_non_existing_env_variable_verbose()
{
var result = CompilationHelper.CompileParams(
("bicepconfig.json", """
{
"analyzers": {
"core": {
"rules": {
},
"verbose": true
}
}
}
"""),
("parameters.bicepparam", @"
using 'foo.bicep'
param fromEnv=readEnvironmentVariable('stringEnvVariable')
"),
("foo.bicep", @"param fromEnv string"));

result.ExcludingLinterDiagnostics().Should().HaveDiagnostics(new[]{
("BCP338", DiagnosticLevel.Error,
"Failed to evaluate parameter \"fromEnv\": Environment variable \"stringEnvVariable\" does not exist, and no default value set."),
("Bicepparam ReadEnvironmentVariable function", DiagnosticLevel.Info,
"Available environment variables are: "
)});
}
[TestMethod]
public void Parameters_file_can_use_variables()
{
Expand Down
3 changes: 3 additions & 0 deletions src/Bicep.Core.UnitTests/Utils/TestEnvironment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,7 @@ public static IEnvironment Create(params (string key, string? value)[] variables

public string? GetVariable(string variable)
=> variables.TryGetValue(variable, out var value) ? value : null;

public IEnumerable<string> GetVariableNames()
=> variables.Keys;
}
22 changes: 20 additions & 2 deletions src/Bicep.Core/Semantics/Namespaces/SystemNamespaceType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using Bicep.Core.Features;
using Bicep.Core.FileSystem;
using Bicep.Core.Intermediate;
using Bicep.Core.Analyzers.Linter;
using Bicep.Core.Modules;
using Bicep.Core.Navigation;
using Bicep.Core.Parsing;
Expand Down Expand Up @@ -1155,9 +1156,26 @@ private static FunctionResult ReadEnvironmentVariableResultBuilder(SemanticModel
}
else
{
//log available environment variables if verbose logging is enabled
if(model.Configuration.Analyzers.GetValue(LinterAnalyzer.LinterEnabledSetting, false) && model.Configuration.Analyzers.GetValue(LinterAnalyzer.LinterVerboseSetting, false)) {
var availableEnvironmentVariables = model.Environment.GetVariableNames();
SimonWahlin marked this conversation as resolved.
Show resolved Hide resolved
diagnostics.Write(
new Diagnostic(
arguments[0].Span,
DiagnosticLevel.Info,
"Bicepparam ReadEnvironmentVariable function",
"Available environment variables are: " + string.Join(", ", model.Environment.GetVariableNames()),
Copy link
Member

Choose a reason for hiding this comment

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

Could this check do something similar to how we handle unknown properties in the type checker? There's a utility method to detect likely typos and include a "did you mean 'x'?" message on

true => SpellChecker.GetSpellingSuggestion(propertyName, availableProperties) switch

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's a great idea!
I'll look into that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done, let me know what you think.

null)
);
}

//error to fail the build-param with clear message of the missing env var name
return new(ErrorType.Create(DiagnosticBuilder.ForPosition(arguments[0]).FailedToEvaluateParameter(envVariableName,
"Environment variable does not exist, and no default value set")));
var paramAssignmentDefinition = model.Root.ParameterAssignments.Where(
p => p.DeclaringParameterAssignment.Value.Span.Position == functionCall.Span.Position
).FirstOrDefault();
var paramName = paramAssignmentDefinition?.Name ?? "";
return new(ErrorType.Create(DiagnosticBuilder.ForPosition(arguments[0]).FailedToEvaluateParameter(paramName,
$"Environment variable \"{envVariableName}\" does not exist, and no default value set.")));
}
}
return new(TypeFactory.CreateStringLiteralType(envVariableValue),
Expand Down
3 changes: 3 additions & 0 deletions src/Bicep.Core/Utils/Environment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,7 @@ public class Environment : IEnvironment
{
public string? GetVariable(string variable)
=> System.Environment.GetEnvironmentVariable(variable);

public IEnumerable<string> GetVariableNames()
=> System.Environment.GetEnvironmentVariables().Keys.Cast<string>();
SimonWahlin marked this conversation as resolved.
Show resolved Hide resolved
}
2 changes: 2 additions & 0 deletions src/Bicep.Core/Utils/IEnvironment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,6 @@ namespace Bicep.Core.Utils;
public interface IEnvironment
{
string? GetVariable(string variable);

IEnumerable<string> GetVariableNames();
}
Loading