Skip to content

Tutorial: Move QL detective tutorial library into shared codeql/tutorial library pack#11747

Merged
adityasharad merged 5 commits intogithub:mainfrom
adityasharad:tutorial/library-pack
Jan 4, 2023
Merged

Tutorial: Move QL detective tutorial library into shared codeql/tutorial library pack#11747
adityasharad merged 5 commits intogithub:mainfrom
adityasharad:tutorial/library-pack

Conversation

@adityasharad
Copy link
Copy Markdown
Collaborator

Key changes:

  • Introduce a new shared pack codeql/tutorial, containing the tutorial.qll library used for the QL detective tutorials.
  • All language library packs codeql/*-all now depend on codeql/tutorial. This means that import tutorial will work in any query that depends on any of the language libraries. Remove the duplicate copies of tutorial.qll.
    • For Go, Ruby, and Swift, this is new behaviour.
    • For other languages, this is existing behaviour.
  • Adjust the QLDoc comments within the tutorial module. The Person class gets a new doc comment, and the tutorial module gets the existing doc comment that was previously attached to Person.

Not in scope in this PR:

  • Changing the import path. This remains import tutorial.
  • Depending on a specific dbscheme. The tutorial library right now has no extractor/dbscheme dependency, and can only be used from a query that does have a transitive dependency on some dbscheme.

@github/code-scanning-secexp-reviewers this should make it easier in future for us to reference the tutorial libraries from a Codespace, but it does not block the existing Codespaces work.

This contains the QL detective tutorial library
in `tutorial.qll`, so that it can be shared by
all language libraries and referenced on its own.
This allows `import tutorial` from queries targeting
any language, just like before, while removing the
duplicate copies of `tutorial.qll`.
By moving the existing doc comment to the top level,
that comment is shown when a user hovers over the module name
in `import tutorial`.
@adityasharad adityasharad marked this pull request as ready for review December 20, 2022 20:45
@adityasharad adityasharad requested review from a team as code owners December 20, 2022 20:45
@adityasharad
Copy link
Copy Markdown
Collaborator Author

Apologies for the wide set of review requests! This touches all language libraries but I hope the changes are focused enough to review for your language separately.

Copy link
Copy Markdown
Contributor

@erik-krogh erik-krogh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It all looks good 👍 I checked all the changes.

At first I thought that you hadn't deleted cpp/ql/lib/tutorial.qll, but that's because it's been renamed to shared/tutorial/tutorial.qll according to git.

upgrades: upgrades
dependencies:
codeql/ssa: ${workspace}
codeql/tutorial: ${workspace}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason for adding a dependency on the tutorial pack? The pack is unused.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Such that you can do import tutorial from any file.

Copy link
Copy Markdown
Contributor

@owen-mc owen-mc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved for Go

@adityasharad adityasharad merged commit ed73875 into github:main Jan 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants