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

Class Y_CHECK_CALL_METHOD_USAGE - Ignore CALL METHOD when classic exceptions are used #72

Closed
ThomasErgin opened this issue Aug 19, 2020 · 8 comments

Comments

@ThomasErgin
Copy link

ThomasErgin commented Aug 19, 2020

The idea of check class Y_CHECK_CALL_METHOD_USAGE is to find outdated method calls invoked with
CALL METHOD myMethod
to encourage developers using the shorter and functional expression
myMethod( ).

However, when using classic exceptions and returning parameters as the same time, you cannot avoid CALL METHOD, e.g.

CALL METHOD myMethod
    RECEIVING = return_value
    EXCEPTIONS others = 1.

There is currently no functional expression for that.

Now you could argument, that classic exceptions should no longer be used. But there are still several common classes in the SAP standard with classic exceptions (e.g. CL_GUI_ALV_GRID). When using these classes the Code Pal would produce pointless warnings. Furthermore, ATC already does provide a check for classic exception.

Following from this, I recommend to skip the CALL METHOD check for all method calls with classic exceptions and return-parameters. In order to do this, the following code change would need to be added to method Y_CHECK_CALL_METHOD_USAGE->INSPECT_TOKENS( ):

...
      AND is_dynamic             = abap_false.

      "Method calls with classic exceptions and returning parameters can not be written in functional style
      LOOP AT ref_scan_manager->get_tokens( ) FROM statement-from TO statement-to REFERENCE INTO DATA(current_token).
        IF current_token->str = 'EXCEPTIONS'.
          DATA(has_classic_exceptions) = abap_true.
        ELSEIF current_token->str = 'RECEIVING'.
          DATA(has_returning_message) = abap_true.
        ENDIF.
      ENDLOOP.

      IF has_classic_exceptions = abap_false OR has_returning_message = abap_false.
        raise_error( p_sub_obj_type = c_type_include
                     p_level        = statement_for_message-level
...

The same applies to the check class Y_CHECK_RECEIVING_USAGE : You must use RECEIVING, when you are working with classic exceptions.

@ghost
Copy link

ghost commented Aug 20, 2020

Hello,

You can write it like in the following example:

REPORT classic_test.

CLASS lcl_classname DEFINITION.
  PUBLIC SECTION.
    METHODS methodname EXCEPTIONS classic_exception.
ENDCLASS.

CLASS lcl_classname IMPLEMENTATION.
  METHOD methodname.
    RAISE classic_exception.
  ENDMETHOD.
ENDCLASS.

START-OF-SELECTION.

  DATA(ce_test) = NEW lcl_classname( ).

  ce_test->methodname( EXCEPTIONS classic_exception = 4 ).
  IF sy-subrc <> 0.
    WRITE: 'Test successful!'.
  ENDIF.

@ghost ghost self-assigned this Aug 20, 2020
@ThomasErgin
Copy link
Author

ThomasErgin commented Aug 20, 2020

Hi Eugen,

The combination of classic exception and returning statement is currently not supported in ABAP:
object->method( EXCEPTIONS classic_exception = 1 ). is allowed, but
DATA(return_value) = myobject->method( EXCEPTIONS classic_exception = 1 ). is not allowed.

So if you want to use both at the same time, you have to use CALL METHOD. I've updated my issue above with this detail.

So I still do think, the checks for RECEIVING and EXCEPTIONS need to be enhanced.

Cheers

Thomas

@ghost
Copy link

ghost commented Aug 20, 2020

its strange that this is even possible.

CLASS lcl_classname DEFINITION.
  PUBLIC SECTION.
    METHODS methodname
      RETURNING  VALUE(result) TYPE abap_bool
      EXCEPTIONS classic_exception.
ENDCLASS.

CLASS lcl_classname IMPLEMENTATION.
  METHOD methodname.
    RAISE classic_exception.
  ENDMETHOD.
ENDCLASS.

But i did not see that kind of combination in the class definition of CL_GUI_ALV_GRID.

Even so i see your point and we will discuss it.

Thank you!

@fabianlupa
Copy link

The combination of classic exception and returning statement is currently not supported in ABAP:
object->method( EXCEPTIONS classic_exception = 1 ). is allowed, but
DATA(return_value) = myobject->method( EXCEPTIONS classic_exception = 1 ). is not allowed.

So if you want to use both at the same time, you have to use CALL METHOD. I've updated my issue above with this detail.

??? Just do this?

object->method(
  RECEIVING
    result            = DATA(lv_result)
  EXCEPTIONS
    classic_exception = 1
).

(Not sure if that triggers the receiving statement usage check to fail.)

@ThomasErgin
Copy link
Author

ThomasErgin commented Aug 21, 2020

Hi Flaiker,

This seems to be a solution - at least to get past the check for usages of CALL METHOD.

But still, the combination of functional passing of a RETURNING-parameter and classic exceptions is not supported by the current netweaver. So
DATA(return_value) = myMethod( EXCEPTIONS thisException = 1 ). won't work and you will run into the check for usage of RECEIVING.

Does anyone of you have contact to the ABAP conpiler development department? Would be great, if this would get into one of the next releases...

Cheers

Thomas

@ghost
Copy link

ghost commented Aug 21, 2020

Hello Thomas,

this way it works just fine:

REPORT classic_test.

CLASS lcl_classname DEFINITION.
  PUBLIC SECTION.
    METHODS methodname
      RETURNING  VALUE(result) TYPE abap_bool
      EXCEPTIONS classic_exception.
ENDCLASS.

CLASS lcl_classname IMPLEMENTATION.
  METHOD methodname.
    RAISE classic_exception.
  ENDMETHOD.
ENDCLASS.

START-OF-SELECTION.

  DATA(ce_test) = NEW lcl_classname( ).

  ce_test->methodname(
                      RECEIVING
                        result            = DATA(data)
                      EXCEPTIONS
                        classic_exception = 4
                      ).

  IF sy-subrc <> 0.
    WRITE: 'Test sucessful!'.
  ENDIF.

The only check we need to adjust is the RECEIVING check.

@ThomasErgin
Copy link
Author

Thank you Eugen!

@fabianlupa
Copy link

Adding support for functional returning in combination with classic exceptions will be impossible because you could use the functional method call in an expression.

DATA(lv_result) = get_a( EXCEPTIONS error = 1 ) + get_b( EXCEPTIONS other_error = 2 ).
IF sy-subrc <> 0.
  " Whose error was raised when????
ENDIF.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants