chore: Merge upstream master into scip-ruby/master - #175
Merged
Merged
Conversation
Bumps [minimatch](https://github.com/isaacs/minimatch) from 3.0.4 to 3.0.5. - [Release notes](https://github.com/isaacs/minimatch/releases) - [Changelog](https://github.com/isaacs/minimatch/blob/main/changelog.md) - [Commits](isaacs/minimatch@v3.0.4...v3.0.5) --- updated-dependencies: - dependency-name: minimatch dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…ss package boundaries (#6653)
* implement basic visibility mechanism * add test case * document the 3723 error * changes * more changes * bump * Update packager/packager.cc Co-authored-by: Nathan Froyd <froydnj@gmail.com> * Update packager/packager.cc Co-authored-by: Nathan Froyd <froydnj@gmail.com> * dedent * tweak comment * test updates * format Co-authored-by: Getty Ritter <gdritter@stripe.com> Co-authored-by: Nathan Froyd <froydnj@gmail.com>
Currently Sorbet runtime matches caller locations against what it assumes are Sorbet runtime source file locations by using a regular expression. The regular expression expects the Sorbet runtime `lib` folder to be directly placed in a folder whose name includes the string `"sorbet-runtime"`. This kind of matching usually works but is prone to breakage if there are other gems that have the string `"sorbet-runtime"` in their folder names, or if the `lib` folder is placed in a folder that is named differently. This commit improves the matching logic by storing the full path to the `lib` folder in a constant, and then filtering caller location paths that start with that path. In order to get the full path to the `lib` folder, we need rely on the const source location of the `T::Private` module, which is almost always guaranteed to be defined in `lib/sorbet_runtime.rb`. The `File.dirname` of that string gives us the full path to the `lib` folder for the running Sorbet runtime gem.
* Reorder URI::File Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com> * Add RBI for URI::WS Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
Co-authored-by: Getty Ritter <gdritter@stripe.com>
Disable runtime type check in CustomType.scalar_type?
* wip: Prototyping `==` errors * Revert "wip: Prototyping `==` errors" This reverts commit b1f16613edfc9f619cbf802aa62543edc24c4926. * wip: Only Symbol#== * wip * wip: Array#include? * wip * put it back * wip BasicObject#== * wip * wip * Add autocorrect exp test * Add error-reference * Fix test * Fix snapshot * Fixes * Forcibly fetch expired yarn key
…onment variables (#6668) * Validate level passed into default_typed_level= * Allow setting `checked_tests` and `default_checked_level` via environment variables * Document environment variables * Fix rubocop * Update gems/sorbet-runtime/README.md * Make tests work even when environment variables are set Co-authored-by: Nathan Froyd <froydnj@gmail.com>
* Allow adding intrinsics to overloaded payload methods Previously, Sorbet would only register the intrinsic on (effectively) the placeholder method that we mangle rename out of the way to make way for overloaded methods. Also previously, we would not store intrinsics in the serialized global state, presumably because it's a bad idea to store pointers there. But now that we're just storing offsets into a fixed-size, consistently-ordered vector, we don't have to do that and can just serialize the intrinsic offset directly. * Add Kernel#raise intrinsic Fixes #4209 * Safeguard against implicit conversions * Add a test * Showcase changes to exp files * Some fixes - Some of these were requests from code review - The change from `auto newArgs` to `DispatchArgs newArgs` was because it caused a santizer "stack-use-after-scope" error that I couldn't solve except by writing it this way :sadpanda: * Add more tests * Make it also handle fail * Add a test for fail * Fix test
Co-authored-by: Nathan Froyd <froydnj@gmail.com>
The `attached_object` method was added to `Class` in Ruby 3.2. his method will return the object for which the receiver is a singleton class, and if the receiver is not a singleton class, it will raise a `TypeError`. For more information, see ruby/ruby#6450 and https://bugs.ruby-lang.org/issues/12084.
* implement export_all! for packages * add test case * tweaks * switch to recursive export_all walk * add comment / delete debug code * refactor a common name lookup method with more cautious classormodule conversions * walk back bit-packing * and a bit more --------- Co-authored-by: Getty Ritter <gdritter@stripe.com>
* Add some performance advice regarding type assertions to docs * typo * Apply suggestions from code review * prettier --------- Co-authored-by: Jake Zimmerman <zimmerman.jake@gmail.com>
* Add test showing existing behavior
The commented out section crashes, but surprisingly, there's no error
for the other part. I don't think people should be relying on it working
that way, and I intend to make that an error.
* Fix crash via findMemberNoDealias
The fix for the crash is one line: `findMember` -> `findMemberNoDealias`
What was happening before was that for an example like this (from the
test suite):
class AbstractRPCMethod
extend T::Generic
RPCInput = type_member
end
class TextDocumentHoverMethod < AbstractRPCMethod
RPCInput = type_template
end
Sorbet thought that it `foundAll` parent type members in the child. That
was deceptive though, because what was actually happening was:
- it found the static field class alias we create in
`TextDocumentHoverMethod` called `RPCInput` that forwards to the
_real_ type_template on `T.class_of(TextDocumentHoverMethod)`
- it just so happened that the thing it resolves to `isTypeMember` and
has the same name, so Sorbet would think that it found what it was
looking for.
I've decided to fix this by not allowing any type members to be
redeclared indirectly via an alias. This prevents some code that used to
work from working, but I can only imagine contrived use cases for that,
and this seems wholly better to ban it.
The error handling code implements a special case for the
type_member/type_template mismatch (not out of necessity but instead to
prevent confusion).
* Some tests change
* Add another test
Fixes #5281
Catch out-of-order constant references behind a cmdline flag Co-authored-by: Jake Zimmerman <zimmerman.jake@gmail.com>
* Backfill constructor tests * Add PORO benchmark * Backfill setter_factory specs * Fix rubocop
* Fixed quoted prop name error message * added double quote test * moved test * added edge cases
* remove String as an option for these * only disallow year --------- Co-authored-by: Getty Ritter <gdritter@stripe.com>
Issue: Current extension code is difficult to follow as it is hevaily entangled, with componens being created in different places and passed around accessed via callbacks. Change: the goal of this PR is to move to a world where there are single purpose classes that are easier to maintain (and eventually test standalone). - `SorbetExtensionContext` is added as a container of references that should be globally accessible. - Logic specific to track the Sorbet Client status is split from the `SorbetStatusBarEntry` class and moved into a new `SorbetStatusProvider` class. This new class exposes events to which the `SorbetStatusBarEntry` class can register to. This helped removing significant amount of entanglement and move and simplify the `<start|stop|restart>Sorbet` methods. - To the extent that it was possible, code was only moved and references updated, without modifying logic in any significant way.
Settings should use `mardownDescription` when using markdown text, instead of `description`, otherwise text is not formatted.
* Change highlight loc of untyped call site * Fixes to the tests
…7053) * Refactor extension logic into purpose-specific components (part 2) This is a follow up to sorbet/sorbet#7046 focused on the "status provider" component: - **Fix**: `filterUpdatesFromOldClients` refactoring was flawed causing status change events to be ignored (e.g. status bar would not reflect current `Typing` state). The new logic removes the method altogether and simplifies the flow significantly, making it far easier to debug. - Move `serverStartus` and `lastError` props from `SorbetStatusBarEntry` to `SorbetStatusProvider`, as the former is intended to be a view-only component, and the latter should be the source of status information. - Reduce refresh rate for the status bar spinner from 100ms to 250ms (this is not an interactive component so high refresh rate is wasteful). * Add missing EOF carriage return. * Bring back `Initializing` state. * Update `onStatusChange` tracking (fix incorrect stale-client filter).
Remove runtime `lodash` dependency as it was brought in to perform a simple `string[]` deep-equality comparison. - Removing this dependency reduces the size of the extension .VSIX package from **3.62MB to 2.59MB**. - Note that `sinon` (used in tests) still depends on `lodash`, so the `yarn.lock` file does not change significantly. ### Motivation The cost-benefit of the library does not add up: 40% increase on the VSIX file to use a single method that is easily implemented with stock JS. Additionally, its presence causes confusion as at least one callsite being removed on this PR used `lodash`'s `isEqual` directly instead of using the more correct class method (that was implemented with it!).
Align code used to capture telemetry events with implementations used inside Stripe. - Intention in sorbet/sorbet#7046 was to create a "telemetry client" , but change was deferred to keep PR smaller. This show why the context's `emitCountMetric` and `emitTimingMetric` existed and are finally remobved. - New implementation removes a risky recursive calls in `emit*` methods. - `venuer` name is an old implementation detail, so renamingfile.
* Add failing test * Fix failing test * Add logic to fix the bad_alias_in_result_type test * Capture master behavior * Show how this test changes * Add another test just to be safe * Update an exp file * Fix test
* prework: Factor out a local variable * prework: Factor out error reporting helper
* pre-work: recvi->loc -> s.recv.loc() * pre-work: Move <Magic> & T handling earlier * pre-work: Factor parseTClassOf helper * pre-work: Return the symbol when parsing T.class_of * pre-work: Shorten error handling * pre-work: Rename to `appliedKlass` for clarity
Fix Command Enablement Rules: - `sorbet.copySymbolToClipboard` and `sorbet.rename` require a Ruby file to be open in an editor to be execut. - If there is no editor, they do nothing. - Today actually the fail if the extension has not been loaded because they are not part of the `activationEvents` list - but adding them is not the right fix (in fact, after VSCode 1.75, commands should not even be there). - `sorbet.showOutput` is disabled in the Command Palette if current file is not Ruby but this is inconsistent with the behavior of other non-editor dependent commands, like `sorbet.configure`. It also breaks the scenario where current editor is a non-Ruby file (e.g. a README file) but the project does contain Ruby files so looking at Sorbet output is still needed. Misc: - Sort command definitions alphabetically by commandId so it is easier to maintain the list.
… but Typescript naming guidelines discourage that, e.g. [Google](https://google.github.io/styleguide/tsguide.html#identifiers-underscore-prefix-suffix ), [TypeScript internal](https://github.com/microsoft/TypeScript/wiki/Coding-guidelines). (#7063) - This can be revisited but this change defines a common baseline and standardizes the codebase. - Getters/setters are usually the one case where `_` helps with naming conflicts but for now adopting [Google](https://google.github.io/styleguide/tsguide.html#properties-used-outside-of-class-lexical-scope)'s recommendation of using a full word prefix like `wrapped` instead (it seems that prefix might be useful during code reviews more than `internal`). This is open to discussion, however, as [Typescript's own handbook](https://www.typescriptlang.org/docs/handbook/2/classes.html#getters--setters) uses an underscore in this case. - Enable `no-underscore-dangle` eslint rule. Misc: - Stopped exporting `deepEqual` from config.ts (feedback from previous PR so fixing as the file is being modified on this PR). - Removed the last two cases of default exports. See https://google.github.io/styleguide/tsguide.html#exports - Updated last cases of files not using camelCase names.
Fix Command Enablement Rules:
- `sorbet.toggleHighlightUntyped` fails when no workspace is open (`_startSorbetProcess` in `LanguageClient` assumes there is `activeLspConfig` all the time):
```
Activating extension 'sorbet.sorbet-vscode-extension' failed:
Cannot read properties of null (reading 'command').
```
- This was meant to be part of sorbet/sorbet#7061.
Structure extension logging based on implementation used by Stripe's internal extensions: - instead of using `OutputChannel.appendLine`, logging methods with different levels (trace, debug, info, warning, error) are used. - log entries now follow the format `«UTC Date Time» [«level»] «message»` (equivalent to one used by VSCode as well as Github and other extensions). - VSCode 1.65 does not provide support to specify a language that would colorize this format automatically - Some internal components request the `OutputChannel` reference to write their own logs, so the format is broken by them (but now difference is evident). - Existing code calling `console.log` was converted into what seemed to be the approrpiate level for each call (i.e. find-and-replaced was NOT used to convert existign calls). - Log levels allow to show/hide information as needed. Extension defaults to write out `info` or higher level messages meaning that now, by default, `Sorbet` output have fewer messages. This can be controlled via - `Sorbet: Set Log Level…` command (similar to VSCode's) . Setting a value this way DOES NOT persist across sessions. - `VSCODE_SORBETEXT_LOG_LEVEL` environment variable . The value is the name of the level, e.g. "debug" and it is case-insensitive.
* updated_prop: `DeprecatedNumeric` → `Numeric` These two constants are aliases of each other in Stripe's codebase. Using `Numeric` is deprecated in props in models in Stripe's codebase. This is normally checked by Rubocop lint rules, but `updated_prop` hides the fact that a prop is using `DeprecatedNumeric`. So whether Sorbet treats this type as `Numeric` or `DeprecatedNumeric` doesn't really matter. For simplicity's sake, let's just say `Numeric`, so that we generate fewer constant literals. `updated_prop` isn't used widely enough for this to make a measurable difference, but at least there's Stripe-specific code in Sorbet. * Update testdata exps
* Remove mentions of Chalk::ODM from Sorbet
There were two mentions of `Chalk::ODM` in Sorbet's implementation:
- `Chalk::ODM::DeprecatedNumeric`, removed as of #7071
- `Chalk::ODM::Document`
It turns out that we got essentially nothing from this change.
Before, if you had a `Chalk::ODM::Document` class, we would generate a
getter for the props in that document that would do
def foo
self.instance_variable_get(:@foo)
end
Otherwise, if it was an unknown class (including any model class),
Sorbet would fall back to the gnarly `self.decorator_class.prop_get`
logic.
However, we flipped that so that it's only enabled in `# compiled: true`
files. Realistically, if we want the Sorbet Compiler to speed up props,
we want it to do so for models, not just documents. So there's a larger
problem here, and I don't think we need to keep around references of
`Chalk::ODM` just to realize a (non-existing) win making prop reads
faster only for Chalk::ODM::Document.
* update testdata exp
This ticket references making `T::Class` generic, which we've done now. There's not much value keeping this test around, as we already wrote tests in that change.
The regression is caused due to b9a7f92, but it isn't super clear to me on how to fix it without reverting the patch (which seems like the wrong thing to do).
varungandhi-src
force-pushed
the
vg/merge-upstream
branch
from
June 7, 2023 04:35
f49c2fe to
c4a8b28
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
n/a
Test plan
Covered by existing tests