diff --git a/doc/commands/abap.md b/doc/commands/abap.md index 140c2aca..884c7bf1 100644 --- a/doc/commands/abap.md +++ b/doc/commands/abap.md @@ -45,12 +45,15 @@ Find more about the options you have when writing your ABAP snippet at: [help.sap.com/adt-class-execution](https://help.sap.com/docs/btp/sap-business-technology-platform/adt-class-execution) ```bash -sapcli abap run [--prefix PREFIX] [--package PACKAGE] SOURCE +sapcli abap run [--prefix PREFIX] [--package PACKAGE] [-D NAME=VALUE] SOURCE ``` * _SOURCE_ path to a file containing ABAP statements, or `-` to read from _stdin_ * _--prefix PREFIX_ class name prefix (default: `zcl_sapcli_run`) * _--package PACKAGE_ package for the temporary class (default: `$tmp`) +* _-D NAME=VALUE, --define NAME=VALUE_ replace `{{NAME}}` tokens in the source + with `VALUE` (repeatable; case sensitive; the last definition of the same + name wins) The temporary class name follows the pattern `__` and is exactly 30 characters long. @@ -63,6 +66,30 @@ be disabled globally via the environment variable purpose - `abap run` is internal orchestration; if the check itself misfires the global env-var is the right knob. +### Preprocessor + +The source is treated as a template: every `{{NAME}}` token is replaced with +the value given via `--define NAME=VALUE` before the code is sent to the +system. Whitespace inside the braces is allowed (`{{ NAME }}`), the token must +not span lines, and names follow the C identifier grammar +(`[A-Za-z_][A-Za-z0-9_]*`, ASCII only). The value may contain `=` - only the +first `=` separates the name from the value. Values are inserted literally; +they are not re-scanned for tokens. Substitution happens everywhere in the +source, including character literals and comments. + +The preprocessor fails with an error instead of sending questionable code to +the system when the source contains: + +* a token without a matching `--define` (a forgotten substitution), +* anything but a plain name between `{{` and `}}` - the content is reserved + for future template features (e.g. Jinja2 style filters), +* the Jinja2 delimiters `{%` or `{#` anywhere in the code - reserved for + future template statements and comments. + +Because the reservation errors are based on the raw text, ABAP code with a +literal `{{`, `{%` or `{#` inside a character literal or a comment is +rejected too; there is no escape syntax yet. + ### Run ABAP from a file ```bash @@ -80,3 +107,9 @@ echo -n "out->write( 'Hello World!' )." | sapcli abap run - ```bash sapcli abap run --prefix zcl_myrun --package '$mypackage' my_script.abap ``` + +### Substitute values in the source + +```bash +echo -n "out->write( '{{GREETING}}, {{WHO}}!' )." | sapcli abap run --define GREETING=Hello --define WHO=World - +``` diff --git a/sap/cli/abap.py b/sap/cli/abap.py index 70d4c71a..2a6cecd3 100644 --- a/sap/cli/abap.py +++ b/sap/cli/abap.py @@ -17,6 +17,28 @@ def __init__(self): super().__init__('abap') +def _parse_definitions(define_args): + """Turns a list of NAME=VALUE strings into a {NAME: VALUE} dict. + + The value may contain '=' (only the first one separates name and value). + The name must follow the preprocessor token name grammar, otherwise the + definition could never match a token. + """ + + definitions = {} + + for item in define_args: + name, separator, value = item.partition('=') + if not separator or not sap.platform.abap.run.TOKEN_NAME_RE.fullmatch(name): + raise sap.errors.SAPCliError(f'Invalid --define value (expected NAME=VALUE): {item}') + + definitions[name] = value + + return definitions + + +@CommandGroup.argument('-D', '--define', type=str, action='append', metavar='NAME=VALUE', + help='Replace {{NAME}} tokens in the source with VALUE (repeatable)') @CommandGroup.argument('--package', type=str, default=sap.platform.abap.run.DEFAULT_PACKAGE) @CommandGroup.argument('--prefix', type=str, default=sap.platform.abap.run.DEFAULT_PREFIX) @CommandGroup.argument('source', type=str) @@ -32,6 +54,8 @@ def run(connection, args): with open(args.source, 'r', encoding='utf-8') as source_file: user_code = source_file.read() + user_code = sap.platform.abap.run.preprocess(user_code, _parse_definitions(args.define or [])) + result = sap.platform.abap.run.execute_abap( connection, user_code, diff --git a/sap/platform/abap/run.py b/sap/platform/abap/run.py index 6da9db02..c58bbf8d 100644 --- a/sap/platform/abap/run.py +++ b/sap/platform/abap/run.py @@ -1,5 +1,6 @@ """ABAP Adhoc Code Execution""" +import re import secrets import string import warnings @@ -16,6 +17,22 @@ _CLASS_DESCRIPTION = 'Temporary class created by sapcli for adhoc ABAP execution' +# Preprocessor token: anything between {{ and }} on a single line. The match is +# deliberately broader than the supported content so that future syntax (defaults, +# filters, functions) fails loudly today instead of being passed to the ABAP system. +_TOKEN_RE = re.compile(r'{{(.*?)}}') + +# Grammar of token names - shared with the --define argument parsing so that every +# definable name is also matchable. Future token syntax will use the characters +# excluded here (e.g. '|', '()') as operators. +TOKEN_NAME_RE = re.compile(r'[A-Za-z_]\w*', re.ASCII) + +# Jinja2 statement and comment delimiters - reserved for future template features. +# Their mere presence is an error (paired or not, Jinja2 comments may span lines), +# so a future upgrade to full Jinja2 templating cannot silently change the meaning +# of sources which work today. +_RESERVED_DELIMITERS = ('{%', '{#') + _CLASS_TEMPLATE = '''"! This is a temporary class created by sapcli for execution "! of an adhoc ABAP statements. CLASS {class_name} DEFINITION @@ -54,6 +71,44 @@ def generate_class_name(prefix, username): return f'{prefix_lower}_{username_lower}_{random_chars}' +def preprocess(user_code, definitions): + """Replaces every {{NAME}} token in user_code with definitions[NAME]. + + Raises SAPCliError if the code contains a token without a matching definition, + so a forgotten substitution fails loudly instead of reaching the ABAP system, + and also if a token holds anything but a plain name - the content between the + braces is reserved for future syntax. The Jinja2 delimiters '{%' and '{#' are + reserved too and must not appear in the code at all. Only the original code is + checked - substituted values are inserted literally and never re-scanned. + """ + + reserved = [delimiter for delimiter in _RESERVED_DELIMITERS if delimiter in user_code] + if reserved: + names = ', '.join(reserved) + raise SAPCliError(f'Reserved preprocessor delimiter(s): {names}') + + undefined = set() + + def _replace(match): + name = match.group(1).strip() + if not TOKEN_NAME_RE.fullmatch(name): + raise SAPCliError(f'Unsupported preprocessor expression: {match.group(0)}') + + if name not in definitions: + undefined.add(name) + return match.group(0) + + return definitions[name] + + result = _TOKEN_RE.sub(_replace, user_code) + + if undefined: + names = ', '.join(sorted(undefined)) + raise SAPCliError(f'Undefined preprocessor token(s): {names}') + + return result + + def build_class_code(class_name, user_code): """Assembles the ABAP class source code wrapping user_code in if_oo_adt_classrun~main""" diff --git a/test/unit/test_sap_cli_abap.py b/test/unit/test_sap_cli_abap.py index 4d299449..efb92033 100644 --- a/test/unit/test_sap_cli_abap.py +++ b/test/unit/test_sap_cli_abap.py @@ -132,6 +132,127 @@ def test_run_custom_package(self): package='$mypackage' ) + def test_run_with_define_substitutes_token(self): + file_content = 'WRITE {{VALUE}}.' + + with patch('builtins.open', mock_open(read_data=file_content)): + mock_exec, _ = self._run_with_mock( + ['run', '--define', 'VALUE=42', 'script.abap'] + ) + + mock_exec.assert_called_once_with( + 'mock_connection', + 'WRITE 42.', + prefix=sap.platform.abap.run.DEFAULT_PREFIX, + package=sap.platform.abap.run.DEFAULT_PACKAGE + ) + + def test_run_with_define_short_flag(self): + file_content = 'WRITE {{VALUE}}.' + + with patch('builtins.open', mock_open(read_data=file_content)): + mock_exec, _ = self._run_with_mock( + ['run', '-D', 'VALUE=42', 'script.abap'] + ) + + mock_exec.assert_called_once_with( + 'mock_connection', + 'WRITE 42.', + prefix=sap.platform.abap.run.DEFAULT_PREFIX, + package=sap.platform.abap.run.DEFAULT_PACKAGE + ) + + def test_run_with_multiple_defines(self): + file_content = '{{A}} {{B}}' + + with patch('builtins.open', mock_open(read_data=file_content)): + mock_exec, _ = self._run_with_mock( + ['run', '--define', 'A=foo', '--define', 'B=bar', 'script.abap'] + ) + + mock_exec.assert_called_once_with( + 'mock_connection', + 'foo bar', + prefix=sap.platform.abap.run.DEFAULT_PREFIX, + package=sap.platform.abap.run.DEFAULT_PACKAGE + ) + + def test_run_without_define_token_in_source_raises(self): + file_content = 'WRITE {{VALUE}}.' + + with patch('builtins.open', mock_open(read_data=file_content)): + args = parse_args(['run', 'script.abap']) + _, factory = make_console_factory() + args.console_factory = factory + + with patch('sap.platform.abap.run.execute_abap') as mock_exec: + with self.assertRaises(SAPCliError) as caught: + args.execute('mock_connection', args) + + mock_exec.assert_not_called() + + self.assertIn('VALUE', str(caught.exception)) + + def test_run_define_value_may_contain_equals(self): + file_content = 'cond = {{EXPR}}.' + + with patch('builtins.open', mock_open(read_data=file_content)): + mock_exec, _ = self._run_with_mock( + ['run', '--define', 'EXPR=a = b', 'script.abap'] + ) + + mock_exec.assert_called_once_with( + 'mock_connection', + 'cond = a = b.', + prefix=sap.platform.abap.run.DEFAULT_PREFIX, + package=sap.platform.abap.run.DEFAULT_PACKAGE + ) + + def test_run_undefined_token_raises_and_skips_execution(self): + file_content = 'WRITE {{MISSING}}.' + + with patch('builtins.open', mock_open(read_data=file_content)): + args = parse_args(['run', '--define', 'OTHER=1', 'script.abap']) + _, factory = make_console_factory() + args.console_factory = factory + + with patch('sap.platform.abap.run.execute_abap') as mock_exec: + with self.assertRaises(SAPCliError) as caught: + args.execute('mock_connection', args) + + mock_exec.assert_not_called() + + self.assertIn('MISSING', str(caught.exception)) + + def test_run_reserved_delimiter_raises_and_skips_execution(self): + file_content = '{% if x %}WRITE.{% endif %}' + + with patch('builtins.open', mock_open(read_data=file_content)): + args = parse_args(['run', 'script.abap']) + _, factory = make_console_factory() + args.console_factory = factory + + with patch('sap.platform.abap.run.execute_abap') as mock_exec: + with self.assertRaises(SAPCliError) as caught: + args.execute('mock_connection', args) + + mock_exec.assert_not_called() + + self.assertIn('{%', str(caught.exception)) + + def test_run_define_invalid_format_raises(self): + file_content = 'WRITE.' + + with patch('builtins.open', mock_open(read_data=file_content)): + args = parse_args(['run', '--define', 'NOEQUALS', 'script.abap']) + _, factory = make_console_factory() + args.console_factory = factory + + with self.assertRaises(SAPCliError) as caught: + args.execute('mock_connection', args) + + self.assertIn('NOEQUALS', str(caught.exception)) + def test_run_prints_output(self): file_content = 'WRITE.' expected_output = 'execution result' @@ -145,6 +266,54 @@ def test_run_prints_output(self): self.assertEqual(console.capout, expected_output + '\n') +class TestParseDefinitions(unittest.TestCase): + + def test_single_definition(self): + self.assertEqual(sap.cli.abap._parse_definitions(['A=1']), {'A': '1'}) + + def test_multiple_definitions(self): + self.assertEqual( + sap.cli.abap._parse_definitions(['A=1', 'B=2']), + {'A': '1', 'B': '2'} + ) + + def test_value_may_contain_equals(self): + self.assertEqual(sap.cli.abap._parse_definitions(['A=1=2']), {'A': '1=2'}) + + def test_empty_value_allowed(self): + self.assertEqual(sap.cli.abap._parse_definitions(['A=']), {'A': ''}) + + def test_later_definition_overrides_earlier(self): + self.assertEqual(sap.cli.abap._parse_definitions(['A=1', 'A=2']), {'A': '2'}) + + def test_missing_equals_raises(self): + with self.assertRaises(SAPCliError): + sap.cli.abap._parse_definitions(['NOEQ']) + + def test_empty_name_raises(self): + with self.assertRaises(SAPCliError): + sap.cli.abap._parse_definitions(['=value']) + + def test_name_with_underscore_allowed(self): + self.assertEqual(sap.cli.abap._parse_definitions(['MY_NAME=1']), {'MY_NAME': '1'}) + + def test_name_with_space_raises(self): + with self.assertRaises(SAPCliError): + sap.cli.abap._parse_definitions(['A B=1']) + + def test_name_with_colon_dash_raises(self): + with self.assertRaises(SAPCliError): + sap.cli.abap._parse_definitions(['A:-B=1']) + + def test_name_with_pipe_raises(self): + with self.assertRaises(SAPCliError): + sap.cli.abap._parse_definitions(['A|B=1']) + + def test_name_starting_with_digit_raises(self): + with self.assertRaises(SAPCliError): + sap.cli.abap._parse_definitions(['1A=2']) + + class TestAbapSystemInfo(unittest.TestCase): def test_systeminfo_prints_all_entries(self): diff --git a/test/unit/test_sap_platform_abap_run.py b/test/unit/test_sap_platform_abap_run.py index 827248b4..6cb88f94 100644 --- a/test/unit/test_sap_platform_abap_run.py +++ b/test/unit/test_sap_platform_abap_run.py @@ -9,6 +9,7 @@ generate_class_name, build_class_code, execute_abap, + preprocess, DEFAULT_PREFIX, DEFAULT_PACKAGE, ) @@ -299,5 +300,126 @@ def test_check_disabled_via_env_skips_check_post(self): self.assertFalse(any(uri.startswith('/sap/bc/adt/checkruns') for _, uri in methods)) +class TestPreprocess(unittest.TestCase): + + def test_replaces_single_token(self): + result = preprocess('WRITE {{VALUE}}.', {'VALUE': '42'}) + self.assertEqual(result, 'WRITE 42.') + + def test_replaces_multiple_occurrences_of_same_token(self): + result = preprocess('{{X}} + {{X}}', {'X': '1'}) + self.assertEqual(result, '1 + 1') + + def test_replaces_multiple_distinct_tokens(self): + result = preprocess('{{A}} {{B}}', {'A': 'foo', 'B': 'bar'}) + self.assertEqual(result, 'foo bar') + + def test_allows_whitespace_inside_braces(self): + result = preprocess('{{ NAME }}', {'NAME': 'x'}) + self.assertEqual(result, 'x') + + def test_code_without_tokens_unchanged(self): + code = 'WRITE / lv_text.' + self.assertEqual(preprocess(code, {'UNUSED': '1'}), code) + + def test_no_definitions_and_no_tokens_returns_same(self): + code = 'WRITE / lv_text.' + self.assertEqual(preprocess(code, {}), code) + + def test_single_braces_are_not_tokens(self): + code = "out->write( |Hello { lv_name }| )." + self.assertEqual(preprocess(code, {'lv_name': 'x'}), code) + + def test_empty_value_replacement(self): + result = preprocess('x{{E}}y', {'E': ''}) + self.assertEqual(result, 'xy') + + def test_value_is_not_interpreted_as_regex(self): + result = preprocess('CALL {{FN}}.', {'FN': "method( '\\1' )"}) + self.assertEqual(result, "CALL method( '\\1' ).") + + def test_undefined_token_raises(self): + with self.assertRaises(SAPCliError) as caught: + preprocess('WRITE {{MISSING}}.', {}) + + self.assertIn('MISSING', str(caught.exception)) + + def test_undefined_token_error_lists_missing_only(self): + with self.assertRaises(SAPCliError) as caught: + preprocess('{{KNOWN}} {{UNKNOWN}}', {'KNOWN': '1'}) + + self.assertIn('UNKNOWN', str(caught.exception)) + + def test_unsupported_expression_bash_default_raises(self): + with self.assertRaises(SAPCliError) as caught: + preprocess("WRITE {{NAME:-'x'}}.", {'NAME': '1'}) + + self.assertIn("{{NAME:-'x'}}", str(caught.exception)) + + def test_unsupported_expression_filter_raises(self): + with self.assertRaises(SAPCliError) as caught: + preprocess("WRITE {{name | default('')}}.", {'name': '1'}) + + self.assertIn('Unsupported preprocessor expression', str(caught.exception)) + + def test_unsupported_expression_function_raises(self): + with self.assertRaises(SAPCliError) as caught: + preprocess('WRITE {{file(name)}}.', {'name': '1'}) + + self.assertIn('Unsupported preprocessor expression', str(caught.exception)) + + def test_unsupported_expression_empty_raises(self): + with self.assertRaises(SAPCliError) as caught: + preprocess('WRITE {{}}.', {}) + + self.assertIn('Unsupported preprocessor expression', str(caught.exception)) + + def test_unsupported_expression_digits_only_raises(self): + with self.assertRaises(SAPCliError) as caught: + preprocess('WRITE {{123}}.', {'123': '1'}) + + self.assertIn('Unsupported preprocessor expression', str(caught.exception)) + + def test_name_starting_with_underscore_is_valid(self): + result = preprocess('WRITE {{_NAME}}.', {'_NAME': '1'}) + self.assertEqual(result, 'WRITE 1.') + + def test_token_must_not_span_lines(self): + code = 'WRITE {{A\nB}} {{C}}.' + result = preprocess(code, {'C': '1'}) + self.assertEqual(result, 'WRITE {{A\nB}} 1.') + + def test_statement_delimiter_is_reserved(self): + with self.assertRaises(SAPCliError) as caught: + preprocess('{% if x %}WRITE.{% endif %}', {}) + + self.assertIn('{%', str(caught.exception)) + + def test_comment_delimiter_is_reserved(self): + with self.assertRaises(SAPCliError) as caught: + preprocess('{# note #}WRITE.', {}) + + self.assertIn('{#', str(caught.exception)) + + def test_unpaired_statement_delimiter_is_reserved(self): + with self.assertRaises(SAPCliError) as caught: + preprocess("WRITE '{%'.", {}) + + self.assertIn('{%', str(caught.exception)) + + def test_reserved_delimiter_error_lists_all_found(self): + with self.assertRaises(SAPCliError) as caught: + preprocess('{# doc #}\n{% if x %}WRITE.{% endif %}', {}) + + self.assertIn('{%', str(caught.exception)) + self.assertIn('{#', str(caught.exception)) + + def test_reserved_delimiter_detected_even_with_valid_tokens(self): + with self.assertRaises(SAPCliError) as caught: + preprocess('{{A}} {% set x = 1 %}', {'A': '1'}) + + self.assertIn('{%', str(caught.exception)) + + if __name__ == '__main__': unittest.main()