ashish10alex / vscode-dataform-tools

Dataform tools - a vscode extension
https://marketplace.visualstudio.com/items?itemName=ashishalex.dataform-lsp-vscode
MIT License
20 stars 4 forks source link

feat: simplify go to definition for js variables #52

Closed ashish10alex closed 1 week ago

ashish10alex commented 1 week ago

This PR:

Test case where the above implementation works

config {
  type: 'table'
}

/*
create a parameters.js file in the includes directory and add 
getDateMonthsBefore function

function getDateMonthsBefore(date, months) {
  date.setMonth(date.getMonth() + months);
  let date_string = date.toISOString().split('T')[0];
  date_string = `'${date_string}'`;
  return date_string

}
*/

js {
  const six_months_ago_from_now = parameters.getDateMonthsBefore(new Date(), -6)
  const four_months_ago_from_now = parameters.getDateMonthsBefore(new Date(), -6)
  const some_month = "january"
  const some_number = 11;
  let some_coniditon = true;
  let another_coniditon = true;
  var some_floating_points = 9.4;
}

select 
  *
, ${six_months_ago_from_now} as some_time
, ${four_months_ago_from_now} as some_time
, ${some_number} as some_number
, ${some_coniditon} as some_condition
, ${another_coniditon} as another_coniditon
, ${some_floating_points} as some_floating_points
, "${some_month}" as some_month 
, ${parameters.OXRC_DATA_STARTING_POINT}
FROM ${ref("0100_CALENDAR")}

Assumptions of the implementation (generous*)

  1. If go to def is triggered from ${some_text} it assumes some_text is variable defined in either a .js file in includes directory or in js block in the same sqlx file 2.If go to def is triggered from ${some_text.other_text} it assumes some_text is the file name and other_text is the variable name that exsists in the .js file
  2. Once we parse out file name and variable name from (2) we use that and search for the 1st occurrence of it. This part needs work as you have suggested in your original MR. As I guess this can be a function too and there might be multiple declarations or use of that word in the js file.
HampB commented 1 week ago

Thanks for the simplifications @ashish10alex —it's looking good! I completely forgot about declarations inside the JavaScript block.

One issue, though: we can't assume the function name is the same as the file name because sometimes imports are from a subdirectory within the includes directory. So, we also need to search the paths of imported modules (getImportedModules) in the findModuleVarDefinition function.

Additionally, we need to search for the variable based on const searchTerm = document.getText(wordRange); (or do some transformation of variableOrFunctionName instead of splitting on the period. When a function has parameters passed to it, like functionName("param1", "param2"), the search won't succeed if it’s split on the period.

I'll merge this PR and make a few small adjustments in the original PR to address these issues.