Skip to content

Commit

Permalink
Lint plugin src (rojo-rbx#846)
Browse files Browse the repository at this point in the history
  • Loading branch information
boatbomber authored Feb 1, 2024
1 parent f3b0b00 commit df707d5
Show file tree
Hide file tree
Showing 18 changed files with 62 additions and 115 deletions.
5 changes: 4 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ jobs:
run: cargo build --locked --verbose

lint:
name: Rustfmt, Clippy, & Stylua
name: Rustfmt, Clippy, Stylua, & Selene
runs-on: ubuntu-latest

steps:
Expand All @@ -98,6 +98,9 @@ jobs:
- name: Stylua
run: stylua --check plugin/src

- name: Selene
run: selene plugin/src

- name: Rustfmt
run: cargo fmt -- --check

Expand Down
2 changes: 1 addition & 1 deletion aftman.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[tools]
rojo = "rojo-rbx/[email protected]"
selene = "Kampfkarren/selene@0.25.0"
selene = "Kampfkarren/selene@0.26.1"
stylua = "JohnnyMorganz/[email protected]"
run-in-roblox = "rojo-rbx/[email protected]"
6 changes: 3 additions & 3 deletions plugin/src/ApiContext.lua
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,10 @@ function ApiContext:write(patch)

body = Http.jsonEncode(body)

return Http.post(url, body):andThen(rejectFailedRequests):andThen(Http.Response.json):andThen(function(body)
Log.info("Write response: {:?}", body)
return Http.post(url, body):andThen(rejectFailedRequests):andThen(Http.Response.json):andThen(function(responseBody)
Log.info("Write response: {:?}", responseBody)

return body
return responseBody
end)
end

Expand Down
2 changes: 1 addition & 1 deletion plugin/src/App/Components/Dropdown.lua
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ function Dropdown:render()
self.setContentSize(object.AbsoluteContentSize)
end,
}),
Roact.createFragment(optionButtons),
Options = Roact.createFragment(optionButtons),
}),
})
else nil,
Expand Down
2 changes: 1 addition & 1 deletion plugin/src/App/Components/PatchVisualizer/DisplayValue.lua
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ local function DisplayValue(props)
-- We don't need to support mixed tables, so checking the first key is enough
-- to determine if it's a simple array
local out, i = table.create(#props.value), 0
for k, v in props.value do
for _, v in props.value do
i += 1

-- Wrap strings in quotes
Expand Down
19 changes: 7 additions & 12 deletions plugin/src/App/Components/PatchVisualizer/DomLabel.lua
Original file line number Diff line number Diff line change
Expand Up @@ -98,21 +98,16 @@ function DomLabel:render()
-- Line guides help indent depth remain readable
local lineGuides = {}
for i = 1, props.depth or 0 do
table.insert(
lineGuides,
e("Frame", {
Name = "Line_" .. i,
Size = UDim2.new(0, 2, 1, 2),
Position = UDim2.new(0, (20 * i) + 15, 0, -1),
BorderSizePixel = 0,
BackgroundTransparency = props.transparency,
BackgroundColor3 = theme.BorderedContainer.BorderColor,
})
)
lineGuides["Line_" .. i] = e("Frame", {
Size = UDim2.new(0, 2, 1, 2),
Position = UDim2.new(0, (20 * i) + 15, 0, -1),
BorderSizePixel = 0,
BackgroundTransparency = props.transparency,
BackgroundColor3 = theme.BorderedContainer.BorderColor,
})
end

return e("Frame", {
Name = "Change",
ClipsDescendants = true,
BackgroundColor3 = if props.patchType then theme.Diff[props.patchType] else nil,
BorderSizePixel = 0,
Expand Down
4 changes: 2 additions & 2 deletions plugin/src/App/Components/TextButton.lua
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ end
function TextButton:render()
return Theme.with(function(theme)
local textSize =
TextService:GetTextSize(self.props.text, 18, Enum.Font.GothamSemibold, Vector2.new(math.huge, math.huge))
TextService:GetTextSize(self.props.text, 18, Enum.Font.GothamMedium, Vector2.new(math.huge, math.huge))

local style = self.props.style

Expand Down Expand Up @@ -83,7 +83,7 @@ function TextButton:render()

Text = e("TextLabel", {
Text = self.props.text,
Font = Enum.Font.GothamSemibold,
Font = Enum.Font.GothamMedium,
TextSize = 18,
TextColor3 = bindingUtil.mapLerp(bindingEnabled, theme.Enabled.TextColor, theme.Disabled.TextColor),
TextTransparency = self.props.transparency,
Expand Down
2 changes: 0 additions & 2 deletions plugin/src/App/StatusPages/Confirming.lua
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
local TextService = game:GetService("TextService")

local Rojo = script:FindFirstAncestor("Rojo")
local Plugin = Rojo.Plugin
local Packages = Rojo.Packages
Expand Down
1 change: 0 additions & 1 deletion plugin/src/App/Theme.lua
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ local Rojo = script:FindFirstAncestor("Rojo")
local Packages = Rojo.Packages

local Roact = require(Packages.Roact)
local Log = require(Packages.Log)

local strict = require(script.Parent.Parent.strict)

Expand Down
16 changes: 8 additions & 8 deletions plugin/src/Reconciler/applyPatch.lua
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@ local function applyPatch(instanceMap, patch)
local unappliedPatch = PatchSet.newEmpty()

for _, removedIdOrInstance in ipairs(patch.removed) do
local ok = pcall(function()
local removeInstanceSuccess = pcall(function()
if Types.RbxId(removedIdOrInstance) then
instanceMap:destroyId(removedIdOrInstance)
else
instanceMap:destroyInstance(removedIdOrInstance)
end
end)
if not ok then
if not removeInstanceSuccess then
table.insert(unappliedPatch.removed, removedIdOrInstance)
end
end
Expand Down Expand Up @@ -175,10 +175,10 @@ local function applyPatch(instanceMap, patch)
end

if update.changedName ~= nil then
local ok = pcall(function()
local setNameSuccess = pcall(function()
instance.Name = update.changedName
end)
if not ok then
if not setNameSuccess then
unappliedUpdate.changedName = update.changedName
partiallyApplied = true
end
Expand All @@ -194,15 +194,15 @@ local function applyPatch(instanceMap, patch)

if update.changedProperties ~= nil then
for propertyName, propertyValue in pairs(update.changedProperties) do
local ok, decodedValue = decodeValue(propertyValue, instanceMap)
if not ok then
local decodeSuccess, decodedValue = decodeValue(propertyValue, instanceMap)
if not decodeSuccess then
unappliedUpdate.changedProperties[propertyName] = propertyValue
partiallyApplied = true
continue
end

local ok = setProperty(instance, propertyName, decodedValue)
if not ok then
local setPropertySuccess = setProperty(instance, propertyName, decodedValue)
if not setPropertySuccess then
unappliedUpdate.changedProperties[propertyName] = propertyValue
partiallyApplied = true
end
Expand Down
4 changes: 2 additions & 2 deletions plugin/src/Reconciler/decodeValue.lua
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ local function decodeValue(encodedValue, instanceMap)
end
end

local ok, decodedValue = RbxDom.EncodedValue.decode(encodedValue)
local decodeSuccess, decodedValue = RbxDom.EncodedValue.decode(encodedValue)

if not ok then
if not decodeSuccess then
return false,
Error.new(Error.CannotDecodeValue, {
encodedValue = encodedValue,
Expand Down
17 changes: 8 additions & 9 deletions plugin/src/Reconciler/diff.lua
Original file line number Diff line number Diff line change
Expand Up @@ -147,13 +147,13 @@ local function diff(instanceMap, virtualInstances, rootId)

local changedProperties = {}
for propertyName, virtualValue in pairs(virtualInstance.Properties) do
local ok, existingValueOrErr = getProperty(instance, propertyName)
local getProperySuccess, existingValueOrErr = getProperty(instance, propertyName)

if ok then
if getProperySuccess then
local existingValue = existingValueOrErr
local ok, decodedValue = decodeValue(virtualValue, instanceMap)
local decodeSuccess, decodedValue = decodeValue(virtualValue, instanceMap)

if ok then
if decodeSuccess then
if not trueEquals(existingValue, decodedValue) then
Log.debug(
"{}.{} changed from '{}' to '{}'",
Expand All @@ -165,7 +165,6 @@ local function diff(instanceMap, virtualInstances, rootId)
changedProperties[propertyName] = virtualValue
end
else
local propertyType = next(virtualValue)
Log.warn(
"Failed to decode property {}.{}. Encoded property was: {:#?}",
virtualInstance.ClassName,
Expand Down Expand Up @@ -220,9 +219,9 @@ local function diff(instanceMap, virtualInstances, rootId)
table.insert(patch.removed, childInstance)
end
else
local ok, err = diffInternal(childId)
local diffSuccess, err = diffInternal(childId)

if not ok then
if not diffSuccess then
return false, err
end
end
Expand All @@ -243,9 +242,9 @@ local function diff(instanceMap, virtualInstances, rootId)
return true
end

local ok, err = diffInternal(rootId)
local diffSuccess, err = diffInternal(rootId)

if not ok then
if not diffSuccess then
return false, err
end

Expand Down
4 changes: 2 additions & 2 deletions plugin/src/Reconciler/hydrate.lua
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ local function hydrate(instanceMap, virtualInstances, rootId, rootInstance)
-- We guard accessing Name and ClassName in order to avoid
-- tripping over children of DataModel that Rojo won't have
-- permissions to access at all.
local ok, name, className = pcall(function()
local accessSuccess, name, className = pcall(function()
return childInstance.Name, childInstance.ClassName
end)

-- This rule is very conservative and could be loosened in the
-- future, or more heuristics could be introduced.
if ok and name == virtualChild.Name and className == virtualChild.ClassName then
if accessSuccess and name == virtualChild.Name and className == virtualChild.ClassName then
isExistingChildVisited[childIndex] = true
hydrate(instanceMap, virtualInstances, childId, childInstance)
break
Expand Down
16 changes: 8 additions & 8 deletions plugin/src/Reconciler/reify.lua
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ function reifyInner(instanceMap, virtualInstances, id, parentInstance, unapplied
-- Instance.new can fail if we're passing in something that can't be
-- created, like a service, something enabled with a feature flag, or
-- something that requires higher security than we have.
local ok, instance = pcall(Instance.new, virtualInstance.ClassName)
local createSuccess, instance = pcall(Instance.new, virtualInstance.ClassName)

if not ok then
if not createSuccess then
addAllToPatch(unappliedPatch, virtualInstances, id)
return
end
Expand All @@ -80,14 +80,14 @@ function reifyInner(instanceMap, virtualInstances, id, parentInstance, unapplied
continue
end

local ok, value = decodeValue(virtualValue, instanceMap)
if not ok then
local decodeSuccess, value = decodeValue(virtualValue, instanceMap)
if not decodeSuccess then
unappliedProperties[propertyName] = virtualValue
continue
end

local ok = setProperty(instance, propertyName, value)
if not ok then
local setPropertySuccess = setProperty(instance, propertyName, value)
if not setPropertySuccess then
unappliedProperties[propertyName] = virtualValue
end
end
Expand Down Expand Up @@ -148,8 +148,8 @@ function applyDeferredRefs(instanceMap, deferredRefs, unappliedPatch)
continue
end

local ok = setProperty(entry.instance, entry.propertyName, targetInstance)
if not ok then
local setPropertySuccess = setProperty(entry.instance, entry.propertyName, targetInstance)
if not setPropertySuccess then
markFailed(entry.id, entry.propertyName, entry.virtualValue)
end
end
Expand Down
1 change: 0 additions & 1 deletion plugin/src/Reconciler/reify.spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ return function()

local PatchSet = require(script.Parent.Parent.PatchSet)
local InstanceMap = require(script.Parent.Parent.InstanceMap)
local Error = require(script.Parent.Error)

local function isEmpty(table)
return next(table) == nil, "Table was not empty"
Expand Down
4 changes: 2 additions & 2 deletions plugin/src/Reconciler/setProperty.lua
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ local function setProperty(instance, propertyName, value)
})
end

local ok, err = descriptor:write(instance, value)
local writeSuccess, err = descriptor:write(instance, value)

if not ok then
if not writeSuccess then
if err.kind == RbxDom.Error.Kind.Roblox and err.extra:find("lacking permission") then
return false,
Error.new(Error.LackingPropertyPermissions, {
Expand Down
19 changes: 13 additions & 6 deletions plugin/src/ServeSession.lua
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ local Status = strict("Session.Status", {
Disconnected = "Disconnected",
})

local function debugPatch(patch)
return Fmt.debugify(patch, function(patch, output)
local function debugPatch(object)
return Fmt.debugify(object, function(patch, output)
output:writeLine("Patch {{")
output:indent()

Expand Down Expand Up @@ -197,7 +197,7 @@ function ServeSession:__onActiveScriptChanged(activeScript)
local existingParent = activeScript.Parent
activeScript.Parent = nil

for i = 1, 3 do
for _ = 1, 3 do
RunService.Heartbeat:Wait()
end

Expand Down Expand Up @@ -251,7 +251,10 @@ function ServeSession:__initialSync(serverInfo)

if userDecision == "Abort" then
return Promise.reject("Aborted Rojo sync operation")
elseif userDecision == "Reject" and self.__twoWaySync then
elseif userDecision == "Reject" then
if not self.__twoWaySync then
return Promise.reject("Cannot reject sync operation without two-way sync enabled")
end
-- The user wants their studio DOM to write back to their Rojo DOM
-- so we will reverse the patch and send it back

Expand All @@ -268,7 +271,7 @@ function ServeSession:__initialSync(serverInfo)
table.insert(inversePatch.updated, update)
end
-- Add the removed instances back to Rojo
-- selene:allow(empty_if, unused_variable)
-- selene:allow(empty_if, unused_variable, empty_loop)
for _, instance in catchUpPatch.removed do
-- TODO: Generate ID for our instance and add it to inversePatch.added
end
Expand All @@ -277,7 +280,7 @@ function ServeSession:__initialSync(serverInfo)
table.insert(inversePatch.removed, id)
end

self.__apiContext:write(inversePatch)
return self.__apiContext:write(inversePatch)
elseif userDecision == "Accept" then
local unappliedPatch = self.__reconciler:applyPatch(catchUpPatch)

Expand All @@ -287,6 +290,10 @@ function ServeSession:__initialSync(serverInfo)
PatchSet.humanSummary(self.__instanceMap, unappliedPatch)
)
end

return Promise.resolve()
else
return Promise.reject("Invalid user decision: " .. userDecision)
end
end)
end
Expand Down
Loading

0 comments on commit df707d5

Please sign in to comment.