From db5dc11b9b27d8fa0b4f9e2d1879829d058c80e8 Mon Sep 17 00:00:00 2001 From: Konstantin Shcheglov Date: Tue, 17 Mar 2020 02:05:41 +0000 Subject: [PATCH] Issue 41050. Get default values for fromEnvironment() from element model. Bug: https://github.com/dart-lang/sdk/issues/41050 Change-Id: I8fee34a6b5816a725df4e4886fc85061ac4ec9b5 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/139661 Reviewed-by: Brian Wilkerson Commit-Queue: Konstantin Shcheglov --- .../lib/src/dart/constant/evaluation.dart | 76 +---------- .../constant/from_environment_evaluator.dart | 129 ++++++++++++++++++ .../from_environment_evaluator_test.dart | 9 ++ .../src/dart/constant/evaluation_test.dart | 98 ++++++++++++- .../const_eval_throws_exception_test.dart | 2 +- 5 files changed, 242 insertions(+), 72 deletions(-) diff --git a/pkg/analyzer/lib/src/dart/constant/evaluation.dart b/pkg/analyzer/lib/src/dart/constant/evaluation.dart index fe7d412eb2f..0a30d829a11 100644 --- a/pkg/analyzer/lib/src/dart/constant/evaluation.dart +++ b/pkg/analyzer/lib/src/dart/constant/evaluation.dart @@ -418,41 +418,6 @@ class ConstantEvaluationEngine { } } - /// Evaluate a call to fromEnvironment() on the bool, int, or String class. - /// The [environmentValue] is the value fetched from the environment. The - /// [builtInDefaultValue] is the value that should be used as the default if - /// no "defaultValue" argument appears in [namedArgumentValues]. The - /// [namedArgumentValues] are the values of the named parameters passed to - /// fromEnvironment(). Return a [DartObjectImpl] object corresponding to the - /// evaluated result. - DartObjectImpl computeValueFromEnvironment( - DartObject environmentValue, - DartObjectImpl builtInDefaultValue, - Map namedArgumentValues) { - DartObjectImpl value = environmentValue as DartObjectImpl; - if (value.isUnknown || value.isNull) { - // The name either doesn't exist in the environment or we couldn't parse - // the corresponding value. - // If the code supplied an explicit default, use it. - if (namedArgumentValues.containsKey(_DEFAULT_VALUE_PARAM)) { - value = namedArgumentValues[_DEFAULT_VALUE_PARAM]; - } else if (value.isNull) { - // The code didn't supply an explicit default. - // The name exists in the environment but we couldn't parse the - // corresponding value. - // So use the built-in default value, because this is what the VM does. - value = builtInDefaultValue; - } else { - // The code didn't supply an explicit default. - // The name doesn't exist in the environment. - // The VM would use the built-in default value, but we don't want to do - // that for analysis because it's likely to lead to cascading errors. - // So just leave [value] in the unknown state. - } - } - return value; - } - DartObjectImpl evaluateConstructorCall( AstNode node, List arguments, @@ -527,43 +492,14 @@ class ConstantEvaluationEngine { String variableName = argumentCount < 1 ? null : argumentValues[0].toStringValue(); if (definingClass == typeProvider.boolType) { - DartObject valueFromEnvironment; - valueFromEnvironment = - _fromEnvironmentEvaluator.getBool(variableName); - return computeValueFromEnvironment( - valueFromEnvironment, - DartObjectImpl( - typeSystem, - typeProvider.boolType, - BoolState.FALSE_STATE, - ), - namedValues, - ); + return _fromEnvironmentEvaluator.getBool2( + variableName, namedValues, constructor); } else if (definingClass == typeProvider.intType) { - DartObject valueFromEnvironment; - valueFromEnvironment = _fromEnvironmentEvaluator.getInt(variableName); - return computeValueFromEnvironment( - valueFromEnvironment, - DartObjectImpl( - typeSystem, - typeProvider.nullType, - NullState.NULL_STATE, - ), - namedValues, - ); + return _fromEnvironmentEvaluator.getInt2( + variableName, namedValues, constructor); } else if (definingClass == typeProvider.stringType) { - DartObject valueFromEnvironment; - valueFromEnvironment = - _fromEnvironmentEvaluator.getString(variableName); - return computeValueFromEnvironment( - valueFromEnvironment, - DartObjectImpl( - typeSystem, - typeProvider.nullType, - NullState.NULL_STATE, - ), - namedValues, - ); + return _fromEnvironmentEvaluator.getString2( + variableName, namedValues, constructor); } } else if (constructor.name == "" && definingClass == typeProvider.symbolType && diff --git a/pkg/analyzer/lib/src/dart/constant/from_environment_evaluator.dart b/pkg/analyzer/lib/src/dart/constant/from_environment_evaluator.dart index 92534ce190d..4e69893c8b8 100644 --- a/pkg/analyzer/lib/src/dart/constant/from_environment_evaluator.dart +++ b/pkg/analyzer/lib/src/dart/constant/from_environment_evaluator.dart @@ -4,10 +4,14 @@ import 'package:analyzer/dart/analysis/declared_variables.dart'; import 'package:analyzer/dart/constant/value.dart'; +import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/src/dart/constant/value.dart'; import 'package:analyzer/src/generated/type_system.dart'; class FromEnvironmentEvaluator { + /// Parameter to "fromEnvironment" methods that denotes the default value. + static const String _defaultValue = 'defaultValue'; + final TypeSystemImpl _typeSystem; final DeclaredVariables _declaredVariables; @@ -17,6 +21,7 @@ class FromEnvironmentEvaluator { /// 'boolean' value. If the variable is not defined (or [name] is `null`), a /// DartObject representing "unknown" is returned. If the value cannot be /// parsed as a boolean, a DartObject representing 'null' is returned. + @Deprecated("Clients don't need this functionality") DartObject getBool(String name) { String value = _declaredVariables.get(name); if (value == null) { @@ -46,10 +51,44 @@ class FromEnvironmentEvaluator { ); } + /// Return the value of the variable with the given [name] interpreted as a + /// 'boolean' value. If the variable is not defined, or the value cannot be + /// parsed as a boolean, return the default value from [namedValues]. If no + /// default value, return the default value of the default value from + /// the [constructor], possibly a [DartObject] representing 'null'. + DartObject getBool2( + String name, + Map namedValues, + ConstructorElement constructor, + ) { + var str = _declaredVariables.get(name); + if (str == 'true') { + return DartObjectImpl( + _typeSystem, + _typeSystem.typeProvider.boolType, + BoolState.TRUE_STATE, + ); + } + if (str == 'false') { + return DartObjectImpl( + _typeSystem, + _typeSystem.typeProvider.boolType, + BoolState.FALSE_STATE, + ); + } + + if (namedValues.containsKey(_defaultValue)) { + return namedValues[_defaultValue]; + } + + return _defaultValueDefaultValue(constructor); + } + /// Return the value of the variable with the given [name] interpreted as an /// integer value. If the variable is not defined (or [name] is `null`), a /// DartObject representing "unknown" is returned. If the value cannot be /// parsed as an integer, a DartObject representing 'null' is returned. + @Deprecated("Clients don't need this functionality") DartObject getInt(String name) { String value = _declaredVariables.get(name); if (value == null) { @@ -76,11 +115,54 @@ class FromEnvironmentEvaluator { ); } + /// Return the value of the variable with the given [name] interpreted as an + /// integer value. If the variable is not defined, or the value cannot be + /// parsed as an integer, return the default value from [namedValues]. If no + /// default value, return the default value of the default value from + /// the [constructor], possibly a [DartObject] representing 'null'. + DartObject getInt2( + String name, + Map namedValues, + ConstructorElement constructor, + ) { + var str = _declaredVariables.get(name); + if (str != null) { + try { + var value = int.parse(str); + return DartObjectImpl( + _typeSystem, + _typeSystem.typeProvider.intType, + IntState(value), + ); + } on FormatException { + // fallthrough + } + } + + if (namedValues.containsKey(_defaultValue)) { + return namedValues[_defaultValue]; + } + + var defaultDefault = _defaultValueDefaultValue(constructor); + + // TODO(scheglov) Remove after https://github.com/dart-lang/sdk/issues/40678 + if (defaultDefault.isNull) { + return DartObjectImpl( + _typeSystem, + _typeSystem.typeProvider.intType, + IntState.UNKNOWN_VALUE, + ); + } + + return defaultDefault; + } + /// Return the value of the variable with the given [name] interpreted as a /// String value, or `null` if the variable is not defined. Return the value /// of the variable with the given name interpreted as a String value. If the /// variable is not defined (or [name] is `null`), a DartObject representing /// "unknown" is returned. + @Deprecated("Clients don't need this functionality") DartObject getString(String name) { String value = _declaredVariables.get(name); if (value == null) { @@ -96,4 +178,51 @@ class FromEnvironmentEvaluator { StringState(value), ); } + + /// Return the value of the variable with the given [name] interpreted as a + /// string value. If the variable is not defined, or the value cannot be + /// parsed as a boolean, return the default value from [namedValues]. If no + /// default value, return the default value of the default value from + /// the [constructor], possibly a [DartObject] representing 'null'. + DartObject getString2( + String name, + Map namedValues, + ConstructorElement constructor, + ) { + String str = _declaredVariables.get(name); + if (str != null) { + return DartObjectImpl( + _typeSystem, + _typeSystem.typeProvider.stringType, + StringState(str), + ); + } + + if (namedValues.containsKey(_defaultValue)) { + return namedValues[_defaultValue]; + } + + var defaultDefault = _defaultValueDefaultValue(constructor); + + // TODO(scheglov) Remove after https://github.com/dart-lang/sdk/issues/40678 + if (defaultDefault.isNull) { + return DartObjectImpl( + _typeSystem, + _typeSystem.typeProvider.stringType, + StringState.UNKNOWN_VALUE, + ); + } + + return defaultDefault; + } + + bool hasEnvironment(String name) { + return _declaredVariables.get(name) != null; + } + + static DartObject _defaultValueDefaultValue(ConstructorElement constructor) { + return constructor.parameters + .singleWhere((parameter) => parameter.name == _defaultValue) + .computeConstantValue(); + } } diff --git a/pkg/analyzer/test/dart/analysis/from_environment_evaluator_test.dart b/pkg/analyzer/test/dart/analysis/from_environment_evaluator_test.dart index 3204b5605a6..5ff1042715a 100644 --- a/pkg/analyzer/test/dart/analysis/from_environment_evaluator_test.dart +++ b/pkg/analyzer/test/dart/analysis/from_environment_evaluator_test.dart @@ -30,6 +30,7 @@ class FromEnvironmentEvaluatorTest { typeSystem = analysisContext.typeSystemLegacy; } + @deprecated void test_getBool_false() { String variableName = "var"; var variables = FromEnvironmentEvaluator( @@ -41,6 +42,7 @@ class FromEnvironmentEvaluatorTest { expect(object.toBoolValue(), false); } + @deprecated void test_getBool_invalid() { String variableName = "var"; var variables = FromEnvironmentEvaluator( @@ -52,6 +54,7 @@ class FromEnvironmentEvaluatorTest { ); } + @deprecated void test_getBool_true() { String variableName = "var"; var variables = FromEnvironmentEvaluator( @@ -63,6 +66,7 @@ class FromEnvironmentEvaluatorTest { expect(object.toBoolValue(), true); } + @deprecated void test_getBool_undefined() { String variableName = "var"; var variables = FromEnvironmentEvaluator( @@ -75,6 +79,7 @@ class FromEnvironmentEvaluatorTest { ); } + @deprecated void test_getInt_invalid() { String variableName = "var"; var variables = FromEnvironmentEvaluator( @@ -86,6 +91,7 @@ class FromEnvironmentEvaluatorTest { ); } + @deprecated void test_getInt_undefined() { String variableName = "var"; var variables = FromEnvironmentEvaluator( @@ -98,6 +104,7 @@ class FromEnvironmentEvaluatorTest { ); } + @deprecated void test_getInt_valid() { String variableName = "var"; var variables = FromEnvironmentEvaluator( @@ -109,6 +116,7 @@ class FromEnvironmentEvaluatorTest { expect(object.toIntValue(), 23); } + @deprecated void test_getString_defined() { String variableName = "var"; String value = "value"; @@ -121,6 +129,7 @@ class FromEnvironmentEvaluatorTest { expect(object.toStringValue(), value); } + @deprecated void test_getString_undefined() { String variableName = "var"; var variables = FromEnvironmentEvaluator( diff --git a/pkg/analyzer/test/src/dart/constant/evaluation_test.dart b/pkg/analyzer/test/src/dart/constant/evaluation_test.dart index 7181255f325..060ddb084e4 100644 --- a/pkg/analyzer/test/src/dart/constant/evaluation_test.dart +++ b/pkg/analyzer/test/src/dart/constant/evaluation_test.dart @@ -108,6 +108,76 @@ const c = true ? x : 0; expect(result, isNull); } + test_visitInstanceCreationExpression_bool_fromEnvironment() async { + await resolveTestCode(''' +const a = bool.fromEnvironment('a'); +const b = bool.fromEnvironment('b', defaultValue: true); +'''); + expect( + _evaluateConstant('a'), + _boolValue(false), + ); + expect( + _evaluateConstant('a', declaredVariables: {'a': 'true'}), + _boolValue(true), + ); + + expect( + _evaluateConstant( + 'b', + declaredVariables: {'b': 'bbb'}, + lexicalEnvironment: {'defaultValue': _boolValue(true)}, + ), + _boolValue(true), + ); + } + + test_visitInstanceCreationExpression_int_fromEnvironment() async { + await resolveTestCode(''' +const a = int.fromEnvironment('a'); +const b = int.fromEnvironment('b', defaultValue: 42); +'''); + expect( + _evaluateConstant('a'), + _intValue(0), + ); + expect( + _evaluateConstant('a', declaredVariables: {'a': '5'}), + _intValue(5), + ); + + expect( + _evaluateConstant( + 'b', + declaredVariables: {'b': 'bbb'}, + lexicalEnvironment: {'defaultValue': _intValue(42)}, + ), + _intValue(42), + ); + } + + test_visitInstanceCreationExpression_string_fromEnvironment() async { + await resolveTestCode(''' +const a = String.fromEnvironment('a'); +'''); + expect( + _evaluateConstant('a'), + DartObjectImpl( + typeSystem, + typeProvider.stringType, + StringState(''), + ), + ); + expect( + _evaluateConstant('a', declaredVariables: {'a': 'test'}), + DartObjectImpl( + typeSystem, + typeProvider.stringType, + StringState('test'), + ), + ); + } + test_visitIntegerLiteral() async { await resolveTestCode(''' const double d = 3; @@ -187,12 +257,38 @@ const b = 3;'''); expect(result.type, typeProvider.intType); expect(result.toIntValue(), 3); } + + DartObjectImpl _boolValue(bool value) { + if (identical(value, false)) { + return DartObjectImpl( + typeSystem, + typeProvider.boolType, + BoolState.FALSE_STATE, + ); + } else if (identical(value, true)) { + return DartObjectImpl( + typeSystem, + typeProvider.boolType, + BoolState.TRUE_STATE, + ); + } + fail("Invalid boolean value used in test"); + } + + DartObjectImpl _intValue(int value) { + return DartObjectImpl( + typeSystem, + typeProvider.intType, + IntState(value), + ); + } } class ConstantVisitorTestSupport extends DriverResolutionTest { DartObjectImpl _evaluateConstant( String name, { List errorCodes, + Map declaredVariables = const {}, Map lexicalEnvironment, }) { var options = driver.analysisOptions as AnalysisOptionsImpl; @@ -210,7 +306,7 @@ class ConstantVisitorTestSupport extends DriverResolutionTest { ConstantVisitor( ConstantEvaluationEngine( typeProvider, - DeclaredVariables(), + DeclaredVariables.fromMap(declaredVariables), experimentStatus: options.experimentStatus, typeSystem: this.result.typeSystem, ), diff --git a/pkg/analyzer/test/src/diagnostics/const_eval_throws_exception_test.dart b/pkg/analyzer/test/src/diagnostics/const_eval_throws_exception_test.dart index 0409d70cd3b..60527d0f773 100644 --- a/pkg/analyzer/test/src/diagnostics/const_eval_throws_exception_test.dart +++ b/pkg/analyzer/test/src/diagnostics/const_eval_throws_exception_test.dart @@ -166,7 +166,7 @@ var x = const C(2); test_fromEnvironment_assertInitializer() async { await assertNoErrorsInCode(''' class A { - const A(int x) : assert(x > 5); + const A(int x) : assert(x >= 0); } main() {