python-rope / rope

a python refactoring library
GNU Lesser General Public License v3.0
1.93k stars 161 forks source link

RemoveArgument - refactoring produces syntax errors at callsite if original code contains comments #690

Open julian-goettingen opened 1 year ago

julian-goettingen commented 1 year ago

Describe the bug When removing an argument from a function, the code at the callsite goes into syntax errors if comments exist at the callsite.

To Reproduce Steps to reproduce the behavior:

  1. Code before refactoring: at the callsite:

    save_list_of_dataframes_to_xlsx(list_of_dfs,
                                        folder_path=folder_path,
                                        # filename=self.__class__.__name__,  # old (GIANNAM, 2021-10-21, 19:02)
                                        filename=self.xlsx_out_filename,     # new (GIANNAM, 2021-10-22, 10:19)
                                        sheetname_list=sheetname_list,
                                        logger=self.logger_obj.logger)
  2. Describe the refactoring you want to do

remove Argument - logger (on the function definition, not the callsite)

  1. Expected code after refactoring:
save_list_of_dataframes_to_xlsx(list_of_dfs,
                                        folder_path=folder_path,
                                        # filename=self.__class__.__name__,  # old (GIANNAM, 2021-10-21, 19:02)
                                        filename=self.xlsx_out_filename,     # new (GIANNAM, 2021-10-22, 10:19)
                                        sheetname_list=sheetname_list)
  1. Describe the error or unexpected result that you are getting

The code gets compressed into one line with the comments mixed in.

 save_list_of_dataframes_to_xlsx(list_of_dfs,folder_path, # filename=self.__class__.__name__,  # old (GIANNAM, 2021-10-21, 19=self.xlsx_out_filename, # new (GIANNAM, 2021-10-22, 10=sheetname_list)

Editor information (please complete the following information):

Additional context still fantastic library thanks a lot

julian-goettingen commented 1 year ago

A shorter example popped up in my code:

before remove logger:

ICE = open_pickle_file(fullfile_path_w_ending=working_file,   # new (GIANNAM, 2022-01-13, 14:08)
                                   settings='rb',
                                   logger=self.logger)

after remove logger: ICE = open_pickle_file(working_file, # new (GIANNAM, 2022-01-13, 14='rb')

expected:

ICE = open_pickle_file(fullfile_path_w_ending=working_file,   # new (GIANNAM, 2022-01-13, 14:08)
                                   settings='rb')
julian-goettingen commented 1 year ago

I have discovered an example where the refactoring breaks syntax directly at the definition-site, also related to comments:

removing the logger_obj from this:

def __init__(self,
                 name=None,
                 save_into_timers_classdict=False,
                 # text="Elapsed time_: {:0.8f} seconds",
                 logger_obj=None,
                 ):

yields this: def __init__(self, , name=None, # text="Elapsed time_=None):

I think there seems to be a general problem with how comments are treated - I realize it is not easy because comments have no ast-equivalent and no c-style /*line-comments*/ exist

lieryan commented 1 year ago

Hi @julian-goettingen, thanks for the report.

Yes, dealing with comments are generally rather tricky to get all the corner cases right, though this one seems to be more on the code generation side.

Squashing everything into a single line here is likely not the appropriate behavior here in any case.