8000 Factor source code-related facilities into a new package by robrix · Pull Request #269 · github/semantic · GitHub
[go: up one dir, main page]

Skip to content
This repository was archived by the owner on Apr 1, 2025. It is now read-only.

Factor source code-related facilities into a new package #269

Merged
merged 49 commits into from
Sep 20, 2019

Conversation

robrix
Copy link
Contributor
@robrix robrix commented Sep 20, 2019

This PR factors Source, Range, Span, Location, &c. out into a new package, semantic-source, intended to be used by semantic, semantic-ast, semantic-tags, semantic-core, semantic-python, and haskell-tree-sitter.

@robrix robrix changed the title Factor source code-related facilities into a new package [WIP] Factor source code-related facilities into a new package Sep 20, 2019
Copy link
Contributor
@patrickt patrickt left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -0,0 +1,21 @@
MIT License

Copyright (c) 2015-2019 GitHub
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A lot of this code has been sitting around since 2015. (At a minimum, the Range stuff.)

Source.Loc
Source.Range
Source.Source
Source.Span
Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO we should format .cabal files this way in general. Aligning crap to the right of labels is terrible.

import Source.Range
import Source.Span

data Loc = Loc
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I opted to abbreviate Location to Loc because we use it everywhere.


data Loc = Loc
{ byteRange :: {-# UNPACK #-} !Range
, span :: {-# UNPACK #-} !Span
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Furthermore, and in keeping with our style guide, I’ve opted to rename its fields to drop the prefix.



byteRange_ :: Lens' Loc Range
byteRange_ = lens byteRange (\l r -> l { byteRange = r })
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also gave it a handy lens to modify its range. We could do a classy-lenses–style HasRange eventually but for now this is it.



instance CustomHasPackageDef Language.Go.Syntax.Package where
customToPackageDef Blob{..} _ (Language.Go.Syntax.Package (Term (In fromAnn _), _) _)
= Just $ PackageDef (getSource fromAnn)
where getSource = toText . flip Source.slice blobSource . locationByteRange
where getSource = toText . Source.slice blobSource . byteRange
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We flipped Source.slice’s definition since we so frequently had to flip its uses instead.

@@ -180,7 +180,7 @@ define :: ( HasCallStack
-> Evaluator term address value m ()
define declaration rel accessControl def = withCurrentCallStack callStack $ do
-- TODO: This span is still wrong.
declare declaration rel accessControl emptySpan Unknown Nothing
declare declaration rel accessControl lowerBound Unknown Nothing
Copy link
Contributor Author

Choose a reason for hiding this comment

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

emptySpan is gone; we rely on the Lower instance instead.

span = lens infoSpan (\i s -> i { infoSpan = s })
{-# INLINE span #-}
span_ = lens infoSpan (\i s -> i { infoSpan = s })
{-# INLINE span_ #-}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We renamed the Span fields to start and end, and now follow the convention that lens names end in _.

toJSONFields sourceSpan = [ "sourceSpan" .= sourceSpan ]

instance ToJSONFields Loc where
toJSONFields Loc{..} = toJSONFields byteRange <> toJSONFields span
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All these instances live here now so we didn’t have to move ToJSONFields into semantic-source or anything.


sourceLineRangesByLineNumber :: Source -> Array Int Range
sourceLineRangesByLineNumber source = listArray (1, length lineRanges) lineRanges
where lineRanges = Source.lineRanges source
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These were only ever being used in this module, and so moving them here prevented having to have semantic-source depend on array.

@robrix robrix changed the title [WIP] Factor source code-related facilities into a new package Factor source code-related facilities into a new package Sep 20, 2019
@robrix
Copy link
Contributor Author
robrix commented Sep 20, 2019

One last bit: we’ve opted to leave Blob in semantic, for now.

@robrix robrix merged commit e96832f into master Sep 20, 2019
@robrix robrix deleted the semantic-source branch September 20, 2019 22:13
@robrix
Copy link
7DA7
Contributor Author
robrix commented Sep 20, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0