IBM / sourceorbit

Dependency Management for IBM i projects
https://ibm.github.io/sourceorbit/
Apache License 2.0
19 stars 10 forks source link

Signed-off-by: Cristian Rivera crisrivlop@gmail.com #48

Closed crisrivlop closed 7 months ago

crisrivlop commented 7 months ago

Resume

Here is an small change for SQLReference detection. Actually SQLReference is only read in the main scope of programs and modules. If there are SQL Sentences called in procedures without any other native sentence (for example: DCL-F or DLC-DS xxx EXTNAME(xxxx)) there is no detection of those SQL sentences. That's why there is a concatenation between the cache.sqlReferences and the sqlReferences from cache.procedures.scope.

Test Repo

I am using the following repo to test this new functionality: Example using source orbit, from this repo the following 2 impact analysis were extracted.

Impact Analysis from main

Impact Analysis

Touched objects:


EMPLOYEE.FILE

Click to expand
* 📁 `EMPLOYEE.FILE` (`qddssrc/employee.table`) Changes to this object have no impact.

Messages

No messages to show.


Project Listing

Click to expand
| - | Object | Type | Path | Warnings | Parents | Children | | --- | --- | --- | --- | --- | --- | --- | | 📁 | DEPTS | FILE | `qddssrc/depts.table` | ✅ | 0 | 0 | | 📁 | EMPLOYEE | FILE | `qddssrc/employee.table` | ✅ | 0 | 0 | | 📦 | EMPSEL | SRVPGM | `qrpglesrc/empsel.bnd` | ✅ |
1$(APP_BNDDIR).BNDDIR
|
1EMPSEL.MOD.MODULE
| | ⛏️ | EMPSEL.MOD | MODULE | `qrpglesrc/empsel.mod.sqlrpgle` | ✅ |
1EMPSEL.SRVPGM
| 0 | | 🛠️ | MYPGM | PGM | `qrpglesrc/mypgm.pgm.sqlrpgle` | ✅ | 0 |
1PGM2.PGM
| | 🛠️ | PGM2 | PGM | `qrpglesrc/pgm2.pgm.sqlrpgle` | ✅ |
1MYPGM.PGM
| 0 | * *Parents* are objects that depend on this object. * *Children* are objects that this object depends on.

Impact Analysis from pull request changes

Impact Analysis

Touched objects:


EMPLOYEE.FILE

Click to expand
* 📁 `EMPLOYEE.FILE` (`qddssrc/employee.table`) * ⛏️ `EMPSEL.MOD.MODULE` (`qrpglesrc/empsel.mod.sqlrpgle`) * 📦 `EMPSEL.SRVPGM` (`qrpglesrc/empsel.bnd`) * 📒 `$(APP_BNDDIR).BNDDIR` (no source) * 🛠️ `PGM2.PGM` (`qrpglesrc/pgm2.pgm.sqlrpgle`) * 🛠️ `MYPGM.PGM` (`qrpglesrc/mypgm.pgm.sqlrpgle`)

Messages

No messages to show.


Project Listing

Click to expand
| - | Object | Type | Path | Warnings | Parents | Children | | --- | --- | --- | --- | --- | --- | --- | | 📁 | DEPTS | FILE | `qddssrc/depts.table` | ✅ |
1EMPSEL.MOD.MODULE
| 0 | | 📁 | EMPLOYEE | FILE | `qddssrc/employee.table` | ✅ |
2EMPSEL.MOD.MODULE, PGM2.PGM
| 0 | | 📦 | EMPSEL | SRVPGM | `qrpglesrc/empsel.bnd` | ✅ |
1$(APP_BNDDIR).BNDDIR
|
1EMPSEL.MOD.MODULE
| | ⛏️ | EMPSEL.MOD | MODULE | `qrpglesrc/empsel.mod.sqlrpgle` | ✅ |
1EMPSEL.SRVPGM
|
2EMPLOYEE.FILE, DEPTS.FILE
| | 🛠️ | MYPGM | PGM | `qrpglesrc/mypgm.pgm.sqlrpgle` | ✅ | 0 |
1PGM2.PGM
| | 🛠️ | PGM2 | PGM | `qrpglesrc/pgm2.pgm.sqlrpgle` | ✅ |
1MYPGM.PGM
|
1EMPLOYEE.FILE
| * *Parents* are objects that depend on this object. * *Children* are objects that this object depends on.
worksofliam commented 7 months ago

Hi @crisrivlop!

First off, thanks so much for your PR! Your change makes sense at a glance and was a complete oversight to me. Here is my review:

crisrivlop commented 7 months ago

Hi @worksofliam!

Thanks for your quick response. I made all the requested changes.

  1. Now npm run test works fine. In order to keep the original test cases I added 1 test files. 1.. sqlReference.test.ts: Contains a simple example showing the new mechanic of sql detection from the procedure scope. It has 2 scenarios: using the sqlreference analysis and without it.
  2. At the same time to isolate this functionality, a new cli option was added -sa/--scope-analysis. This was made to keep the consistency of the current scenarios.
    so -bf imd -sa -l qddssrc/employee.table

    I appreciate your help with your feedback!

worksofliam commented 7 months ago

@crisrivlop Great work here! Thank you greatly for your test case!

I am also finding additional edge cases that I have not covered (my bad!), so with your permission do you mind if I also add some commits to your PR? I also think there are ways to make it simpler.

Let me know if you're okay with that.

crisrivlop commented 7 months ago

Hi! @worksofliam, yes sure it is ok for me :)

worksofliam commented 7 months ago

@crisrivlop

I have made my commit. Do a pull and let me know what you think.

Here is the gist of my changes:

crisrivlop commented 7 months ago

It seems to be a better solution and is solving more issues than SQL references. So we can continue with the change :)

worksofliam commented 7 months ago

Thanks @crisrivlop ! I will publish a new version of the CLI today with this fix.

I am grateful for your PR!